From 779441f42d93141b89da7d5e3cb633af7c74c042 Mon Sep 17 00:00:00 2001 From: mmenanno <1358708+mmenanno@users.noreply.github.com> Date: Mon, 13 Oct 2025 11:44:14 -0400 Subject: [PATCH 1/4] refactor(notifications): replace webhook KeyMap with LiquidJS templating MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the bespoke KeyMap substitution in the webhook agent with a shared LiquidJS-based TemplateEngine, laying groundwork for templating across all notification agents. - Add TemplateEngine (server/lib/notifications/templateEngine.ts) wrapping LiquidJS. Liquid provides conditionals, comparisons, and filters (default, upcase, truncate, json, url_encode, …) natively, so no custom helpers are needed. Liquid does not HTML-escape output, so apostrophes etc. render verbatim. The built-in json filter is overridden to be circular-safe and to log rather than drop data silently. - buildContext exposes the same scalar variables as the previous KeyMap (including discordIds, media_imdbid, jellyfinMediaId, plexRatingKey and requestedBy_jellyfinUserId) plus the full media/request/issue/comment/extra values for conditionals. - webhook agent renders the stored template to a string and lets JSON.parse validate the result; no regex payload cleanup. customHeaders support is retained. - Relax server-side and client-side payload validation: the payload is now a Liquid template that need not be valid JSON until rendered. The default payload uses {% if %}…{% else %}null{% endif %} blocks so optional sections stay null when absent (matching prior behavior), and emits free-text values (titles, overviews, display names, comments) through the json filter so characters like quotes and newlines cannot break the surrounding JSON. - URL variable substitution renders raw; use the url_encode filter where encoding is needed. - Add unit tests for TemplateEngine (escaping, optional-section null handling, circular-safe json filter, and buildContext variable mapping). --- package.json | 1 + pnpm-lock.yaml | 18 ++ server/lib/notifications/agents/webhook.ts | 154 ++-------------- .../lib/notifications/templateEngine.test.ts | 171 ++++++++++++++++++ server/lib/notifications/templateEngine.ts | 160 ++++++++++++++++ server/routes/settings/notifications.ts | 4 - .../NotificationsWebhook/index.tsx | 131 ++++++-------- 7 files changed, 424 insertions(+), 215 deletions(-) create mode 100644 server/lib/notifications/templateEngine.test.ts create mode 100644 server/lib/notifications/templateEngine.ts diff --git a/package.json b/package.json index dc15c7b88b..f143e4fab9 100644 --- a/package.json +++ b/package.json @@ -68,6 +68,7 @@ "humanize-duration": "^3.33.2", "ip-address": "^10.7.5", "js-yaml": "^4.3.2", + "liquidjs": "^10.27.0", "lodash": "4.18.1", "lru-cache": "11.2.1", "mime": "^4.1.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 32a3836716..ddd26b5c29 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -115,6 +115,9 @@ importers: js-yaml: specifier: ^4.3.2 version: 4.3.2 + liquidjs: + specifier: ^10.27.0 + version: 10.30.1 lodash: specifier: 4.18.1 version: 4.18.1 @@ -3311,6 +3314,10 @@ packages: comma-separated-tokens@2.0.3: resolution: {integrity: sha512-Fu4hJdvzeylCfQPp9SGWidpzrMs7tTrlu6Vb8XGaRGck8QSNZJJp538Wrb60Lax4fPwR64ViY468OIUTbRlGZg==} + commander@10.0.1: + resolution: {integrity: sha512-y4Mg2tXshplEbSGzx7amzPwKKOCGuoSRP/CjEdwwk0FOGlUbq6lKuoyDZTNZkmxHdJtp54hdfY/JUrdL7Xfdug==} + engines: {node: '>=14'} + commander@12.1.0: resolution: {integrity: sha512-Vw8qHK3bZM9y/P10u3Vib8o/DdkvA2OtPtZvD871QKjy74Wj1WSKFILMPRPSdUSx5RFK1arlJzEtA4PkFgnbuA==} engines: {node: '>=18'} @@ -4971,6 +4978,11 @@ packages: engines: {node: '>=20.17'} hasBin: true + liquidjs@10.30.1: + resolution: {integrity: sha512-P1Iz14WUHZdfbOYwHOIhtl/iAWIOYk1ks5l9rPZlDM6vZo+6BLeYIuWHAPkIz+U8Mm8HKZKITIrjX7DQJO2fqA==} + engines: {node: '>=16'} + hasBin: true + listr2@9.0.5: resolution: {integrity: sha512-ME4Fb83LgEgwNw96RKNvKV4VTLuXfoKudAmm2lP8Kk87KaMK0/Xrx/aAkMWmT8mDb+3MlFDspfbCs7adjRxA2g==} engines: {node: '>=20.0.0'} @@ -10425,6 +10437,8 @@ snapshots: comma-separated-tokens@2.0.3: {} + commander@10.0.1: {} + commander@12.1.0: {} commander@14.0.3: {} @@ -12466,6 +12480,10 @@ snapshots: tinyexec: 1.1.1 yaml: 2.9.1 + liquidjs@10.30.1: + dependencies: + commander: 10.0.1 + listr2@9.0.5: dependencies: cli-truncate: 5.2.0 diff --git a/server/lib/notifications/agents/webhook.ts b/server/lib/notifications/agents/webhook.ts index 3fdde03ab8..43e666bca0 100644 --- a/server/lib/notifications/agents/webhook.ts +++ b/server/lib/notifications/agents/webhook.ts @@ -1,68 +1,12 @@ -import { IssueStatus, IssueType } from '@server/constants/issue'; -import { MediaStatus } from '@server/constants/media'; +import { TemplateEngine } from '@server/lib/notifications/templateEngine'; import type { NotificationAgentWebhook } from '@server/lib/settings'; import { getSettings } from '@server/lib/settings'; import logger from '@server/logger'; import axios from 'axios'; -import { get } from 'lodash'; import { Notification, hasNotificationType } from '..'; import type { NotificationAgent, NotificationPayload } from './agent'; import { BaseAgent } from './agent'; -type KeyMapFunction = ( - payload: NotificationPayload, - type: Notification -) => string; - -const KeyMap: Record = { - notification_type: (_payload, type) => Notification[type], - event: 'event', - subject: 'subject', - message: 'message', - image: 'image', - notifyuser_username: 'notifyUser.displayName', - notifyuser_email: 'notifyUser.email', - notifyuser_avatar: 'notifyUser.avatar', - notifyuser_settings_discordIds: 'notifyUser.settings.discordIds', - notifyuser_settings_telegramChatId: 'notifyUser.settings.telegramChatId', - media_imdbid: 'media.imdbId', - media_tmdbid: 'media.tmdbId', - media_tvdbid: 'media.tvdbId', - media_type: 'media.mediaType', - media_jellyfinMediaId: (payload) => - payload.media?.jellyfinMediaId ?? payload.media?.jellyfinMediaId4k ?? '', - media_status: (payload) => - payload.media ? MediaStatus[payload.media.status] : '', - media_status4k: (payload) => - payload.media ? MediaStatus[payload.media.status4k] : '', - media_plexRatingKey: (payload) => payload.media?.ratingKey ?? '', - media_plexRatingKey4k: (payload) => payload.media?.ratingKey4k ?? '', - request_id: 'request.id', - requestedBy_jellyfinUserId: 'request.requestedBy.jellyfinUserId', - requestedBy_username: 'request.requestedBy.displayName', - requestedBy_email: 'request.requestedBy.email', - requestedBy_avatar: 'request.requestedBy.avatar', - requestedBy_settings_discordIds: 'request.requestedBy.settings.discordIds', - requestedBy_settings_telegramChatId: - 'request.requestedBy.settings.telegramChatId', - issue_id: 'issue.id', - issue_type: (payload) => - payload.issue ? IssueType[payload.issue.issueType] : '', - issue_status: (payload) => - payload.issue ? IssueStatus[payload.issue.status] : '', - reportedBy_username: 'issue.createdBy.displayName', - reportedBy_email: 'issue.createdBy.email', - reportedBy_avatar: 'issue.createdBy.avatar', - reportedBy_settings_discordIds: 'issue.createdBy.settings.discordIds', - reportedBy_settings_telegramChatId: 'issue.createdBy.settings.telegramChatId', - comment_message: 'comment.message', - commentedBy_username: 'comment.user.displayName', - commentedBy_email: 'comment.user.email', - commentedBy_avatar: 'comment.user.avatar', - commentedBy_settings_discordIds: 'comment.user.settings.discordIds', - commentedBy_settings_telegramChatId: 'comment.user.settings.telegramChatId', -}; - class WebhookAgent extends BaseAgent implements NotificationAgent @@ -77,81 +21,18 @@ class WebhookAgent return settings.notifications.agents.webhook; } - private parseKeys( - finalPayload: Record, - payload: NotificationPayload, - type: Notification - ): Record { - Object.keys(finalPayload).forEach((key) => { - if (key === '{{extra}}') { - finalPayload.extra = payload.extra ?? []; - delete finalPayload[key]; - key = 'extra'; - } else if (key === '{{media}}') { - if (payload.media) { - finalPayload.media = finalPayload[key]; - } else { - finalPayload.media = null; - } - delete finalPayload[key]; - key = 'media'; - } else if (key === '{{request}}') { - if (payload.request) { - finalPayload.request = finalPayload[key]; - } else { - finalPayload.request = null; - } - delete finalPayload[key]; - key = 'request'; - } else if (key === '{{issue}}') { - if (payload.issue) { - finalPayload.issue = finalPayload[key]; - } else { - finalPayload.issue = null; - } - delete finalPayload[key]; - key = 'issue'; - } else if (key === '{{comment}}') { - if (payload.comment) { - finalPayload.comment = finalPayload[key]; - } else { - finalPayload.comment = null; - } - delete finalPayload[key]; - key = 'comment'; - } - - if (typeof finalPayload[key] === 'string') { - Object.keys(KeyMap).forEach((keymapKey) => { - const keymapValue = KeyMap[keymapKey as keyof typeof KeyMap]; - finalPayload[key] = (finalPayload[key] as string).replace( - `{{${keymapKey}}}`, - typeof keymapValue === 'function' - ? keymapValue(payload, type) - : (get(payload, keymapValue) ?? '') - ); - }); - } else if (finalPayload[key] && typeof finalPayload[key] === 'object') { - finalPayload[key] = this.parseKeys( - finalPayload[key] as Record, - payload, - type - ); - } - }); - - return finalPayload; - } - private buildPayload(type: Notification, payload: NotificationPayload) { const payloadString = Buffer.from( this.getSettings().options.jsonPayload, 'base64' ).toString('utf8'); - const parsedJSON = JSON.parse(JSON.parse(payloadString)); + // The payload is stored as a JSON-encoded string; decode it to the raw + // template, render it, then parse the result as the final JSON body. + const template = JSON.parse(payloadString); + const rendered = TemplateEngine.render(template, payload, type); - return this.parseKeys(parsedJSON, payload, type); + return JSON.parse(rendered); } public shouldSend(): boolean { @@ -186,19 +67,18 @@ class WebhookAgent let webhookUrl = settings.options.webhookUrl; if (settings.options.supportVariables) { - Object.keys(KeyMap).forEach((keymapKey) => { - const keymapValue = KeyMap[keymapKey as keyof typeof KeyMap]; - const variableValue = - type === Notification.TEST_NOTIFICATION - ? 'test' - : typeof keymapValue === 'function' - ? keymapValue(payload, type) - : get(payload, keymapValue) || 'test'; - webhookUrl = webhookUrl.replace( - new RegExp(`{{${keymapKey}}}`, 'g'), - encodeURIComponent(variableValue) + try { + webhookUrl = TemplateEngine.render(webhookUrl, payload, type); + } catch (error) { + logger.warn( + 'Error rendering webhook URL template, using original URL', + { + label: 'Notifications', + errorMessage: + error instanceof Error ? error.message : 'Unknown error', + } ); - }); + } } try { diff --git a/server/lib/notifications/templateEngine.test.ts b/server/lib/notifications/templateEngine.test.ts new file mode 100644 index 0000000000..64c0338d68 --- /dev/null +++ b/server/lib/notifications/templateEngine.test.ts @@ -0,0 +1,171 @@ +import { IssueStatus, IssueType } from '@server/constants/issue'; +import { MediaStatus } from '@server/constants/media'; +import { Notification } from '@server/lib/notifications'; +import type { NotificationPayload } from '@server/lib/notifications/agents/agent'; +import { TemplateEngine } from '@server/lib/notifications/templateEngine'; +import assert from 'node:assert/strict'; +import { describe, it } from 'node:test'; + +const makePayload = ( + overrides: Partial = {} +): NotificationPayload => + ({ + subject: 'Test Subject', + notifySystem: true, + notifyAdmin: false, + ...overrides, + }) as unknown as NotificationPayload; + +describe('TemplateEngine', () => { + describe('render', () => { + it('escapes free-text values piped through the json filter', () => { + const payload = makePayload({ + subject: 'The "Best" Movie', + message: 'line one\nline two with a backslash \\', + }); + + const rendered = TemplateEngine.render( + '{ "subject": {{ subject | json }}, "message": {{ message | json }} }', + payload, + Notification.TEST_NOTIFICATION + ); + + const parsed = JSON.parse(rendered); + assert.equal(parsed.subject, 'The "Best" Movie'); + assert.equal(parsed.message, 'line one\nline two with a backslash \\'); + }); + + it('renders optional sections as null when the entity is absent', () => { + const template = + '{ "media": {% if media %}{ "tmdbId": "{{ media_tmdbid }}" }{% else %}null{% endif %} }'; + + const absent = JSON.parse( + TemplateEngine.render( + template, + makePayload(), + Notification.MEDIA_PENDING + ) + ); + assert.equal(absent.media, null); + + const present = JSON.parse( + TemplateEngine.render( + template, + makePayload({ media: { tmdbId: 42 } as never }), + Notification.MEDIA_PENDING + ) + ); + assert.deepEqual(present.media, { tmdbId: '42' }); + }); + + it('does not crash on circular references in the json filter', () => { + const circular: Record = {}; + circular.self = circular; + + const rendered = TemplateEngine.render( + '{{ extra | json }}', + makePayload({ extra: circular as never }), + Notification.TEST_NOTIFICATION + ); + + assert.equal(rendered, 'null'); + }); + }); + + describe('buildContext', () => { + it('maps the notification type to its enum name', () => { + const context = TemplateEngine.buildContext( + makePayload(), + Notification.MEDIA_AUTO_APPROVED + ); + + assert.equal(context.notification_type, 'MEDIA_AUTO_APPROVED'); + }); + + it('exposes media fields and converts status enums to names', () => { + const context = TemplateEngine.buildContext( + makePayload({ + media: { + tmdbId: 603, + tvdbId: 1234, + imdbId: 'tt0133093', + mediaType: 'movie', + status: MediaStatus.AVAILABLE, + status4k: MediaStatus.PENDING, + } as never, + }), + Notification.MEDIA_AVAILABLE + ); + + assert.equal(context.media_tmdbid, 603); + assert.equal(context.media_imdbid, 'tt0133093'); + assert.equal(context.media_type, 'movie'); + assert.equal(context.media_status, 'AVAILABLE'); + assert.equal(context.media_status4k, 'PENDING'); + }); + + it('defaults absent scalar fields to empty strings', () => { + const context = TemplateEngine.buildContext( + makePayload(), + Notification.TEST_NOTIFICATION + ); + + assert.equal(context.media_tmdbid, ''); + assert.equal(context.media_status, ''); + assert.equal(context.request_id, ''); + assert.equal(context.comment_message, ''); + }); + + it('maps notifyuser_* to notifyUser only, without falling back to the requester', () => { + const context = TemplateEngine.buildContext( + makePayload({ + request: { + id: 7, + requestedBy: { displayName: 'Requester' }, + } as never, + }), + Notification.MEDIA_PENDING + ); + + // notifyUser is intentionally undefined for admin notifications; the + // requester must not leak into notifyuser_* variables. + assert.equal(context.notifyuser_username, ''); + assert.equal(context.requestedBy_username, 'Requester'); + }); + + it('converts issue enums to names', () => { + const context = TemplateEngine.buildContext( + makePayload({ + issue: { + id: 3, + issueType: IssueType.VIDEO, + status: IssueStatus.OPEN, + } as never, + }), + Notification.ISSUE_CREATED + ); + + assert.equal(context.issue_type, 'VIDEO'); + assert.equal(context.issue_status, 'OPEN'); + }); + + it('exposes full entity values as null when absent and the raw value when present', () => { + const absent = TemplateEngine.buildContext( + makePayload(), + Notification.TEST_NOTIFICATION + ); + assert.equal(absent.media, null); + assert.equal(absent.request, null); + assert.equal(absent.issue, null); + assert.equal(absent.comment, null); + assert.deepEqual(absent.extra, []); + + const media = { tmdbId: 1 } as never; + const present = TemplateEngine.buildContext( + makePayload({ media }), + Notification.MEDIA_AVAILABLE + ); + assert.equal(present.media, media); + }); + }); +}); diff --git a/server/lib/notifications/templateEngine.ts b/server/lib/notifications/templateEngine.ts new file mode 100644 index 0000000000..604034bd59 --- /dev/null +++ b/server/lib/notifications/templateEngine.ts @@ -0,0 +1,160 @@ +import { IssueStatus, IssueType } from '@server/constants/issue'; +import { MediaStatus } from '@server/constants/media'; +import logger from '@server/logger'; +import { Liquid } from 'liquidjs'; +import { get } from 'lodash'; +import type { NotificationPayload } from './agents/agent'; +import { Notification } from './index'; + +/** + * LiquidJS-based templating for notification agents. Note that Liquid does not + * HTML-escape output, so characters such as apostrophes and quotes render + * verbatim. + */ +export class TemplateEngine { + private static engine: Liquid | undefined; + + private static getEngine(): Liquid { + if (!this.engine) { + const engine = new Liquid(); + + // Make the `json` filter circular-safe: TypeORM entities can hold + // circular relations (e.g. Media <-> Issue) that JSON.stringify rejects. + engine.registerFilter('json', (value: unknown) => { + try { + return JSON.stringify(value); + } catch (error) { + logger.warn('Unable to serialize value in webhook json filter', { + label: 'Notifications', + errorMessage: + error instanceof Error ? error.message : 'Unknown error', + }); + return 'null'; + } + }); + + this.engine = engine; + } + + return this.engine; + } + + /** + * Builds the template context: flat scalar variables for substitution, plus + * the full `media`/`request`/`issue`/`comment`/`extra` values for + * conditionals (e.g. `{% if media %}`). + */ + static buildContext( + payload: NotificationPayload, + type: Notification + ): Record { + return { + notification_type: Notification[type], + event: payload.event ?? '', + subject: payload.subject ?? '', + message: payload.message ?? '', + image: payload.image ?? '', + + notifyuser_username: get(payload, 'notifyUser.displayName', ''), + notifyuser_email: get(payload, 'notifyUser.email', ''), + notifyuser_avatar: get(payload, 'notifyUser.avatar', ''), + notifyuser_settings_discordIds: get( + payload, + 'notifyUser.settings.discordIds', + '' + ), + notifyuser_settings_telegramChatId: get( + payload, + 'notifyUser.settings.telegramChatId', + '' + ), + + media_imdbid: get(payload, 'media.imdbId', ''), + media_tmdbid: get(payload, 'media.tmdbId', ''), + media_tvdbid: get(payload, 'media.tvdbId', ''), + media_type: get(payload, 'media.mediaType', ''), + media_jellyfinMediaId: + payload.media?.jellyfinMediaId ?? + payload.media?.jellyfinMediaId4k ?? + '', + media_status: payload.media ? MediaStatus[payload.media.status] : '', + media_status4k: payload.media ? MediaStatus[payload.media.status4k] : '', + media_plexRatingKey: payload.media?.ratingKey ?? '', + media_plexRatingKey4k: payload.media?.ratingKey4k ?? '', + + request_id: get(payload, 'request.id', ''), + requestedBy_jellyfinUserId: get( + payload, + 'request.requestedBy.jellyfinUserId', + '' + ), + requestedBy_username: get(payload, 'request.requestedBy.displayName', ''), + requestedBy_email: get(payload, 'request.requestedBy.email', ''), + requestedBy_avatar: get(payload, 'request.requestedBy.avatar', ''), + requestedBy_settings_discordIds: get( + payload, + 'request.requestedBy.settings.discordIds', + '' + ), + requestedBy_settings_telegramChatId: get( + payload, + 'request.requestedBy.settings.telegramChatId', + '' + ), + + issue_id: get(payload, 'issue.id', ''), + issue_type: payload.issue ? IssueType[payload.issue.issueType] : '', + issue_status: payload.issue ? IssueStatus[payload.issue.status] : '', + reportedBy_username: get(payload, 'issue.createdBy.displayName', ''), + reportedBy_email: get(payload, 'issue.createdBy.email', ''), + reportedBy_avatar: get(payload, 'issue.createdBy.avatar', ''), + reportedBy_settings_discordIds: get( + payload, + 'issue.createdBy.settings.discordIds', + '' + ), + reportedBy_settings_telegramChatId: get( + payload, + 'issue.createdBy.settings.telegramChatId', + '' + ), + + comment_message: get(payload, 'comment.message', ''), + commentedBy_username: get(payload, 'comment.user.displayName', ''), + commentedBy_email: get(payload, 'comment.user.email', ''), + commentedBy_avatar: get(payload, 'comment.user.avatar', ''), + commentedBy_settings_discordIds: get( + payload, + 'comment.user.settings.discordIds', + '' + ), + commentedBy_settings_telegramChatId: get( + payload, + 'comment.user.settings.telegramChatId', + '' + ), + + // Full values, exposed for conditionals and advanced templates + extra: payload.extra ?? [], + media: payload.media ?? null, + request: payload.request ?? null, + issue: payload.issue ?? null, + comment: payload.comment ?? null, + }; + } + + /** + * Renders a Liquid template string against a notification payload. + * + * @throws if the template contains invalid Liquid syntax + */ + static render( + template: string, + payload: NotificationPayload, + type: Notification + ): string { + const context = this.buildContext(payload, type); + // parseAndRenderSync is typed `any`; coerce to honor the `string` return. + return String(this.getEngine().parseAndRenderSync(template, context)); + } +} diff --git a/server/routes/settings/notifications.ts b/server/routes/settings/notifications.ts index f987b9e3d0..94b1f82b43 100644 --- a/server/routes/settings/notifications.ts +++ b/server/routes/settings/notifications.ts @@ -300,8 +300,6 @@ notificationRoutes.get('/webhook', (_req, res) => { notificationRoutes.post('/webhook', async (req, res, next) => { const settings = getSettings(); try { - JSON.parse(req.body.options.jsonPayload); - settings.notifications.agents.webhook = { enabled: req.body.enabled, embedPoster: req.body.embedPoster, @@ -333,8 +331,6 @@ notificationRoutes.post('/webhook/test', async (req, res, next) => { } try { - JSON.parse(req.body.options.jsonPayload); - const testBody = { enabled: req.body.enabled, embedPoster: req.body.embedPoster, diff --git a/src/components/Settings/Notifications/NotificationsWebhook/index.tsx b/src/components/Settings/Notifications/NotificationsWebhook/index.tsx index d69a6c9c4f..6d8bb74b37 100644 --- a/src/components/Settings/Notifications/NotificationsWebhook/index.tsx +++ b/src/components/Settings/Notifications/NotificationsWebhook/index.tsx @@ -29,53 +29,54 @@ const JSONEditor = dynamic(() => import('@app/components/JSONEditor'), { ssr: false, }); -const defaultPayload = { - notification_type: '{{notification_type}}', - event: '{{event}}', - subject: '{{subject}}', - message: '{{message}}', - image: '{{image}}', - '{{media}}': { - media_type: '{{media_type}}', - imdbId: '{{media_imdbid}}', - tmdbId: '{{media_tmdbid}}', - tvdbId: '{{media_tvdbid}}', - jellyfinMediaId: '{{media_jellyfinMediaId}}', - status: '{{media_status}}', - status4k: '{{media_status4k}}', - }, - '{{request}}': { - request_id: '{{request_id}}', - requestedBy_email: '{{requestedBy_email}}', - requestedBy_username: '{{requestedBy_username}}', - requestedBy_avatar: '{{requestedBy_avatar}}', - requestedBy_jellyfinUserId: '{{requestedBy_jellyfinUserId}}', - requestedBy_settings_discordIds: '{{requestedBy_settings_discordIds}}', - requestedBy_settings_telegramChatId: - '{{requestedBy_settings_telegramChatId}}', - }, - '{{issue}}': { - issue_id: '{{issue_id}}', - issue_type: '{{issue_type}}', - issue_status: '{{issue_status}}', - reportedBy_email: '{{reportedBy_email}}', - reportedBy_username: '{{reportedBy_username}}', - reportedBy_avatar: '{{reportedBy_avatar}}', - reportedBy_settings_discordIds: '{{reportedBy_settings_discordIds}}', - reportedBy_settings_telegramChatId: - '{{reportedBy_settings_telegramChatId}}', - }, - '{{comment}}': { - comment_message: '{{comment_message}}', - commentedBy_email: '{{commentedBy_email}}', - commentedBy_username: '{{commentedBy_username}}', - commentedBy_avatar: '{{commentedBy_avatar}}', - commentedBy_settings_discordIds: '{{commentedBy_settings_discordIds}}', - commentedBy_settings_telegramChatId: - '{{commentedBy_settings_telegramChatId}}', - }, - '{{extra}}': [], -}; +// Liquid template rendered into the webhook payload. Optional sections use +// `{% if %}…{% else %}null{% endif %}` so they render as null (not omitted) +// when absent, and free-text values use the `json` filter to quote and escape +// themselves so characters like `"` or newlines can't break the JSON. +const defaultPayload = `{ + "notification_type": "{{ notification_type }}", + "event": "{{ event }}", + "subject": {{ subject | json }}, + "message": {{ message | json }}, + "image": "{{ image }}", + "media": {% if media %}{ + "media_type": "{{ media_type }}", + "imdbId": "{{ media_imdbid }}", + "tmdbId": "{{ media_tmdbid }}", + "tvdbId": "{{ media_tvdbid }}", + "jellyfinMediaId": "{{ media_jellyfinMediaId }}", + "status": "{{ media_status }}", + "status4k": "{{ media_status4k }}" + }{% else %}null{% endif %}, + "request": {% if request %}{ + "request_id": "{{ request_id }}", + "requestedBy_email": "{{ requestedBy_email }}", + "requestedBy_username": {{ requestedBy_username | json }}, + "requestedBy_avatar": "{{ requestedBy_avatar }}", + "requestedBy_jellyfinUserId": "{{ requestedBy_jellyfinUserId }}", + "requestedBy_settings_discordIds": "{{ requestedBy_settings_discordIds | join: ',' }}", + "requestedBy_settings_telegramChatId": "{{ requestedBy_settings_telegramChatId }}" + }{% else %}null{% endif %}, + "issue": {% if issue %}{ + "issue_id": "{{ issue_id }}", + "issue_type": "{{ issue_type }}", + "issue_status": "{{ issue_status }}", + "reportedBy_email": "{{ reportedBy_email }}", + "reportedBy_username": {{ reportedBy_username | json }}, + "reportedBy_avatar": "{{ reportedBy_avatar }}", + "reportedBy_settings_discordIds": "{{ reportedBy_settings_discordIds | join: ',' }}", + "reportedBy_settings_telegramChatId": "{{ reportedBy_settings_telegramChatId }}" + }{% else %}null{% endif %}, + "comment": {% if comment %}{ + "comment_message": {{ comment_message | json }}, + "commentedBy_email": "{{ commentedBy_email }}", + "commentedBy_username": {{ commentedBy_username | json }}, + "commentedBy_avatar": "{{ commentedBy_avatar }}", + "commentedBy_settings_discordIds": "{{ commentedBy_settings_discordIds | join: ',' }}", + "commentedBy_settings_telegramChatId": "{{ commentedBy_settings_telegramChatId }}" + }{% else %}null{% endif %}, + "extra": {{ extra | json }} +}`; const messages = defineMessages( 'components.Settings.Notifications.NotificationsWebhook', @@ -181,29 +182,14 @@ const NotificationsWebhook = () => { } ), - jsonPayload: Yup.string() - .when('enabled', { - is: true, - then: (schema) => - schema - .nullable() - .required( - intl.formatMessage(messages.validationJsonPayloadRequired) - ), - otherwise: (schema) => schema.nullable(), - }) - .test( - 'validate-json', - intl.formatMessage(messages.validationJsonPayloadRequired), - (value) => { - try { - JSON.parse(value ?? ''); - return true; - } catch { - return false; - } - } - ), + jsonPayload: Yup.string().when('enabled', { + is: true, + then: (schema) => + schema + .nullable() + .required(intl.formatMessage(messages.validationJsonPayloadRequired)), + otherwise: (schema) => schema.nullable(), + }), }); if (!data && !error) { @@ -267,10 +253,7 @@ const NotificationsWebhook = () => { setFieldTouched, }) => { const resetPayload = () => { - setFieldValue( - 'jsonPayload', - JSON.stringify(defaultPayload, undefined, ' ') - ); + setFieldValue('jsonPayload', defaultPayload); addToast(intl.formatMessage(messages.resetPayloadSuccess), { appearance: 'info', autoDismiss: true, From b6b8f8f64e1e11b075c447ec3464e68dc30d915f Mon Sep 17 00:00:00 2001 From: mmenanno <1358708+mmenanno@users.noreply.github.com> Date: Sat, 4 Jul 2026 15:38:30 -0400 Subject: [PATCH 2/4] refactor(notifications): harden webhook templating and simplify payload storage Addresses review feedback on the LiquidJS webhook refactor: - Fail closed when a webhook URL or payload template fails to render, logging a clear error instead of POSTing to an unrendered URL. - Store the payload as a raw base64 template (drop the JSON.stringify/parse envelope) and add migration 0009 to re-encode existing settings. - Fix the server-side default payload to the LiquidJS template so fresh installs render valid JSON. - Document LiquidJS template syntax and link the NotificationPayload interface; mark the flat variables as deprecated. - Add a test covering undefined template variables. --- docs/using-seerr/notifications/webhook.md | 75 +++++++++++++++---- server/lib/notifications/agents/webhook.ts | 39 ++++++---- .../lib/notifications/templateEngine.test.ts | 12 +++ server/lib/settings/index.ts | 56 +++++++++++++- .../0009_migrate_webhook_payload_encoding.ts | 41 ++++++++++ server/routes/settings/notifications.ts | 21 +++--- 6 files changed, 203 insertions(+), 41 deletions(-) create mode 100644 server/lib/settings/migrations/0009_migrate_webhook_payload_encoding.ts diff --git a/docs/using-seerr/notifications/webhook.md b/docs/using-seerr/notifications/webhook.md index c68b557c45..235f275f6d 100644 --- a/docs/using-seerr/notifications/webhook.md +++ b/docs/using-seerr/notifications/webhook.md @@ -39,10 +39,57 @@ You cannot configure both the **Authorization Header** field and a custom `Autho ### JSON Payload -Customize the JSON payload to suit your needs. Seerr provides several [template variables](#template-variables) for use in the payload, which will be replaced with the relevant data when the notifications are triggered. +Customize the JSON payload to suit your needs. The payload is a [Liquid](https://liquidjs.com/) template that is rendered when a notification is triggered and then sent as the request body. Seerr exposes a set of [template variables](#template-variables), plus the full `media`, `request`, `issue`, and `comment` objects, for use in [conditionals and filters](#template-syntax). + +The template only needs to _render_ to valid JSON — it does not need to be valid JSON itself (for example, optional sections can be wrapped in `{% if %}` blocks). Use the **Test** button to confirm your template renders correctly; template errors are also written to the server logs. + +## Template Syntax + +Payloads and webhook URLs are rendered with [LiquidJS](https://liquidjs.com/), so in addition to simple `{{ variable }}` substitution you can use Liquid tags and filters. + +### Conditionals + +Render a section only when the relevant object is present, falling back to `null` otherwise: + +``` +"media": {% if media %}{ "tmdbId": "{{ media_tmdbid }}" }{% else %}null{% endif %} +``` + +You can also branch on a value: + +``` +{% if media_status == "AVAILABLE" %}...{% else %}...{% endif %} +``` + +### Filters + +- **`json`** — `{{ subject | json }}`: serializes a value as JSON, quoting and escaping strings so characters like `"` or newlines can't break the payload. Circular-safe. +- **`default`** — `{{ notifyuser_username | default: "Unknown User" }}`: falls back to a value when the input is nil or empty. +- **`upcase` / `downcase`** — `{{ media_type | upcase }}`: changes case. +- **`capitalize`** — `{{ notifyuser_username | capitalize }}`: capitalizes the first character. +- **`truncate`** — `{{ subject | truncate: 50 }}`: shortens a string. +- **`url_encode`** — `{{ subject | url_encode }}`: percent-encodes a value for use in a URL. + +See the [LiquidJS filter reference](https://liquidjs.com/filters/overview.html) for the full list. + +:::info +Free-text values (titles, overviews, usernames, comments) should be emitted through the `json` filter so quotes and newlines can't produce invalid JSON, e.g. `"subject": {{ subject | json }}`. +::: + +:::warning +Webhook URL values are rendered raw. When substituting a value into a URL, use the `url_encode` filter (e.g. `...?user={{ notifyuser_username | url_encode }}`) so characters like `&`, `?`, or spaces don't break the URL. +::: + +### Advanced object access + +In addition to the flat variables documented below, the full `media`, `request`, `issue`, and `comment` objects are available for conditionals and advanced templates (for example `{% if issue %}` or `{{ request.id }}`). These map to the [`NotificationPayload` interface](https://github.com/seerr-team/seerr/blob/develop/server/lib/notifications/agents/agent.ts). ## Template Variables +:::warning Deprecated +The flat template variables listed below (e.g. `{{ media_tmdbid }}`, `{{ notifyuser_username }}`) are deprecated and will be replaced in a future release. They continue to work for now, and a migration will be provided to convert existing templates automatically. +::: + ### General | Variable | Value | @@ -81,21 +128,21 @@ On the other hand, the `notifyuser` variables _will_ be replaced with the reques If you would like to use the requesting user's information in your webhook, please instead include the relevant variables from the [Request](#request) section below. ::: -### Special +### Objects -The following variables must be used as a key in the JSON payload (e.g., `"{{extra}}": []`). +The following values are the full objects for each notification. They are `null` when not relevant to the notification, and are primarily useful in conditionals (e.g. `{% if media %}`) or for accessing fields not covered by the flat variables below. Serialize an object into the payload with the `json` filter, e.g. `"extra": {{ extra | json }}`. -| Variable | Value | -| ------------- | ------------------------------------------------------------------------------------------------------------------------------ | -| `{{media}}` | The relevant media object | -| `{{request}}` | The relevant request object | -| `{{issue}}` | The relevant issue object | -| `{{comment}}` | The relevant issue comment object | -| `{{extra}}` | The "extra" array of additional data for certain notifications (e.g., season/episode numbers for series-related notifications) | +| Variable | Value | +| ----------- | ------------------------------------------------------------------------------------------------------------------------------ | +| `media` | The relevant media object | +| `request` | The relevant request object | +| `issue` | The relevant issue object | +| `comment` | The relevant issue comment object | +| `extra` | The "extra" array of additional data for certain notifications (e.g., season/episode numbers for series-related notifications) | ### Media -The `{{media}}` will be `null` if there is no relevant media object for the notification. +`media` will be `null` if there is no relevant media object for the notification. These following special variables are only included in media-related notifications, such as requests. @@ -113,7 +160,7 @@ These following special variables are only included in media-related notificatio ### Request -The `{{request}}` will be `null` if there is no relevant media object for the notification. +`request` will be `null` if there is no relevant request object for the notification. The following special variables are only included in request-related notifications. @@ -129,7 +176,7 @@ The following special variables are only included in request-related notificatio ### Issue -The `{{issue}}` will be `null` if there is no relevant media object for the notification. +`issue` will be `null` if there is no relevant issue object for the notification. The following special variables are only included in issue-related notifications. @@ -144,7 +191,7 @@ The following special variables are only included in issue-related notifications ### Comment -The `{{comment}}` will be `null` if there is no relevant media object for the notification. +`comment` will be `null` if there is no relevant comment object for the notification. The following special variables are only included in issue comment-related notifications. diff --git a/server/lib/notifications/agents/webhook.ts b/server/lib/notifications/agents/webhook.ts index 43e666bca0..98ab7487d8 100644 --- a/server/lib/notifications/agents/webhook.ts +++ b/server/lib/notifications/agents/webhook.ts @@ -22,14 +22,10 @@ class WebhookAgent } private buildPayload(type: Notification, payload: NotificationPayload) { - const payloadString = Buffer.from( + const template = Buffer.from( this.getSettings().options.jsonPayload, 'base64' ).toString('utf8'); - - // The payload is stored as a JSON-encoded string; decode it to the raw - // template, render it, then parse the result as the final JSON body. - const template = JSON.parse(payloadString); const rendered = TemplateEngine.render(template, payload, type); return JSON.parse(rendered); @@ -70,17 +66,32 @@ class WebhookAgent try { webhookUrl = TemplateEngine.render(webhookUrl, payload, type); } catch (error) { - logger.warn( - 'Error rendering webhook URL template, using original URL', - { - label: 'Notifications', - errorMessage: - error instanceof Error ? error.message : 'Unknown error', - } - ); + logger.error('Failed to render webhook URL template', { + label: 'Notifications', + type: Notification[type], + subject: payload.subject, + errorMessage: + error instanceof Error ? error.message : 'Unknown error', + }); + + return false; } } + let body: unknown; + try { + body = this.buildPayload(type, payload); + } catch (error) { + logger.error('Failed to render webhook payload template', { + label: 'Notifications', + type: Notification[type], + subject: payload.subject, + errorMessage: error instanceof Error ? error.message : 'Unknown error', + }); + + return false; + } + try { const headers: Record = {}; @@ -110,7 +121,7 @@ class WebhookAgent await axios.post( webhookUrl, - this.buildPayload(type, payload), + body, Object.keys(headers).length > 0 ? { headers } : undefined ); diff --git a/server/lib/notifications/templateEngine.test.ts b/server/lib/notifications/templateEngine.test.ts index 64c0338d68..811bbf1d18 100644 --- a/server/lib/notifications/templateEngine.test.ts +++ b/server/lib/notifications/templateEngine.test.ts @@ -58,6 +58,18 @@ describe('TemplateEngine', () => { assert.deepEqual(present.media, { tmdbId: '42' }); }); + it('renders undefined variables as empty strings without throwing', () => { + const rendered = TemplateEngine.render( + '{ "known": {{ subject | json }}, "unknown": "{{ nonexistent_variable }}" }', + makePayload({ subject: 'Known Value' }), + Notification.TEST_NOTIFICATION + ); + + const parsed = JSON.parse(rendered); + assert.equal(parsed.known, 'Known Value'); + assert.equal(parsed.unknown, ''); + }); + it('does not crash on circular references in the json filter', () => { const circular: Record = {}; circular.self = circular; diff --git a/server/lib/settings/index.ts b/server/lib/settings/index.ts index a52f8cf4c6..2bf8f6ea9e 100644 --- a/server/lib/settings/index.ts +++ b/server/lib/settings/index.ts @@ -397,6 +397,57 @@ const SETTINGS_PATH = process.env.CONFIG_DIRECTORY ? `${process.env.CONFIG_DIRECTORY}/settings.json` : path.join(__dirname, '../../../config/settings.json'); +// Default webhook payload, rendered through the LiquidJS template engine. +// Optional sections use `{% if %}…{% else %}null{% endif %}` so they render as +// null (not omitted) when absent, and free-text values use the `json` filter to +// quote/escape themselves. Keep this byte-identical to the frontend +// `defaultPayload` in +// src/components/Settings/Notifications/NotificationsWebhook/index.tsx. +const DEFAULT_WEBHOOK_JSON_PAYLOAD = `{ + "notification_type": "{{ notification_type }}", + "event": "{{ event }}", + "subject": {{ subject | json }}, + "message": {{ message | json }}, + "image": "{{ image }}", + "media": {% if media %}{ + "media_type": "{{ media_type }}", + "imdbId": "{{ media_imdbid }}", + "tmdbId": "{{ media_tmdbid }}", + "tvdbId": "{{ media_tvdbid }}", + "jellyfinMediaId": "{{ media_jellyfinMediaId }}", + "status": "{{ media_status }}", + "status4k": "{{ media_status4k }}" + }{% else %}null{% endif %}, + "request": {% if request %}{ + "request_id": "{{ request_id }}", + "requestedBy_email": "{{ requestedBy_email }}", + "requestedBy_username": {{ requestedBy_username | json }}, + "requestedBy_avatar": "{{ requestedBy_avatar }}", + "requestedBy_jellyfinUserId": "{{ requestedBy_jellyfinUserId }}", + "requestedBy_settings_discordIds": "{{ requestedBy_settings_discordIds | join: ',' }}", + "requestedBy_settings_telegramChatId": "{{ requestedBy_settings_telegramChatId }}" + }{% else %}null{% endif %}, + "issue": {% if issue %}{ + "issue_id": "{{ issue_id }}", + "issue_type": "{{ issue_type }}", + "issue_status": "{{ issue_status }}", + "reportedBy_email": "{{ reportedBy_email }}", + "reportedBy_username": {{ reportedBy_username | json }}, + "reportedBy_avatar": "{{ reportedBy_avatar }}", + "reportedBy_settings_discordIds": "{{ reportedBy_settings_discordIds | join: ',' }}", + "reportedBy_settings_telegramChatId": "{{ reportedBy_settings_telegramChatId }}" + }{% else %}null{% endif %}, + "comment": {% if comment %}{ + "comment_message": {{ comment_message | json }}, + "commentedBy_email": "{{ commentedBy_email }}", + "commentedBy_username": {{ commentedBy_username | json }}, + "commentedBy_avatar": "{{ commentedBy_avatar }}", + "commentedBy_settings_discordIds": "{{ commentedBy_settings_discordIds | join: ',' }}", + "commentedBy_settings_telegramChatId": "{{ commentedBy_settings_telegramChatId }}" + }{% else %}null{% endif %}, + "extra": {{ extra | json }} +}`; + class Settings { private data: AllSettings; private saveLock: Promise = Promise.resolve(); @@ -540,8 +591,9 @@ class Settings { types: 0, options: { webhookUrl: '', - jsonPayload: - 'IntcbiAgXCJub3RpZmljYXRpb25fdHlwZVwiOiBcInt7bm90aWZpY2F0aW9uX3R5cGV9fVwiLFxuICBcImV2ZW50XCI6IFwie3tldmVudH19XCIsXG4gIFwic3ViamVjdFwiOiBcInt7c3ViamVjdH19XCIsXG4gIFwibWVzc2FnZVwiOiBcInt7bWVzc2FnZX19XCIsXG4gIFwiaW1hZ2VcIjogXCJ7e2ltYWdlfX1cIixcbiAgXCJ7e21lZGlhfX1cIjoge1xuICAgIFwibWVkaWFfdHlwZVwiOiBcInt7bWVkaWFfdHlwZX19XCIsXG4gICAgXCJ0bWRiSWRcIjogXCJ7e21lZGlhX3RtZGJpZH19XCIsXG4gICAgXCJ0dmRiSWRcIjogXCJ7e21lZGlhX3R2ZGJpZH19XCIsXG4gICAgXCJzdGF0dXNcIjogXCJ7e21lZGlhX3N0YXR1c319XCIsXG4gICAgXCJzdGF0dXM0a1wiOiBcInt7bWVkaWFfc3RhdHVzNGt9fVwiXG4gIH0sXG4gIFwie3tyZXF1ZXN0fX1cIjoge1xuICAgIFwicmVxdWVzdF9pZFwiOiBcInt7cmVxdWVzdF9pZH19XCIsXG4gICAgXCJyZXF1ZXN0ZWRCeV9lbWFpbFwiOiBcInt7cmVxdWVzdGVkQnlfZW1haWx9fVwiLFxuICAgIFwicmVxdWVzdGVkQnlfdXNlcm5hbWVcIjogXCJ7e3JlcXVlc3RlZEJ5X3VzZXJuYW1lfX1cIixcbiAgICBcInJlcXVlc3RlZEJ5X2F2YXRhclwiOiBcInt7cmVxdWVzdGVkQnlfYXZhdGFyfX1cIixcbiAgICBcInJlcXVlc3RlZEJ5X3NldHRpbmdzX2Rpc2NvcmRJZFwiOiBcInt7cmVxdWVzdGVkQnlfc2V0dGluZ3NfZGlzY29yZElkfX1cIixcbiAgICBcInJlcXVlc3RlZEJ5X3NldHRpbmdzX3RlbGVncmFtQ2hhdElkXCI6IFwie3tyZXF1ZXN0ZWRCeV9zZXR0aW5nc190ZWxlZ3JhbUNoYXRJZH19XCJcbiAgfSxcbiAgXCJ7e2lzc3VlfX1cIjoge1xuICAgIFwiaXNzdWVfaWRcIjogXCJ7e2lzc3VlX2lkfX1cIixcbiAgICBcImlzc3VlX3R5cGVcIjogXCJ7e2lzc3VlX3R5cGV9fVwiLFxuICAgIFwiaXNzdWVfc3RhdHVzXCI6IFwie3tpc3N1ZV9zdGF0dXN9fVwiLFxuICAgIFwicmVwb3J0ZWRCeV9lbWFpbFwiOiBcInt7cmVwb3J0ZWRCeV9lbWFpbH19XCIsXG4gICAgXCJyZXBvcnRlZEJ5X3VzZXJuYW1lXCI6IFwie3tyZXBvcnRlZEJ5X3VzZXJuYW1lfX1cIixcbiAgICBcInJlcG9ydGVkQnlfYXZhdGFyXCI6IFwie3tyZXBvcnRlZEJ5X2F2YXRhcn19XCIsXG4gICAgXCJyZXBvcnRlZEJ5X3NldHRpbmdzX2Rpc2NvcmRJZFwiOiBcInt7cmVwb3J0ZWRCeV9zZXR0aW5nc19kaXNjb3JkSWR9fVwiLFxuICAgIFwicmVwb3J0ZWRCeV9zZXR0aW5nc190ZWxlZ3JhbUNoYXRJZFwiOiBcInt7cmVwb3J0ZWRCeV9zZXR0aW5nc190ZWxlZ3JhbUNoYXRJZH19XCJcbiAgfSxcbiAgXCJ7e2NvbW1lbnR9fVwiOiB7XG4gICAgXCJjb21tZW50X21lc3NhZ2VcIjogXCJ7e2NvbW1lbnRfbWVzc2FnZX19XCIsXG4gICAgXCJjb21tZW50ZWRCeV9lbWFpbFwiOiBcInt7Y29tbWVudGVkQnlfZW1haWx9fVwiLFxuICAgIFwiY29tbWVudGVkQnlfdXNlcm5hbWVcIjogXCJ7e2NvbW1lbnRlZEJ5X3VzZXJuYW1lfX1cIixcbiAgICBcImNvbW1lbnRlZEJ5X2F2YXRhclwiOiBcInt7Y29tbWVudGVkQnlfYXZhdGFyfX1cIixcbiAgICBcImNvbW1lbnRlZEJ5X3NldHRpbmdzX2Rpc2NvcmRJZFwiOiBcInt7Y29tbWVudGVkQnlfc2V0dGluZ3NfZGlzY29yZElkfX1cIixcbiAgICBcImNvbW1lbnRlZEJ5X3NldHRpbmdzX3RlbGVncmFtQ2hhdElkXCI6IFwie3tjb21tZW50ZWRCeV9zZXR0aW5nc190ZWxlZ3JhbUNoYXRJZH19XCJcbiAgfSxcbiAgXCJ7e2V4dHJhfX1cIjogW11cbn0i', + jsonPayload: Buffer.from(DEFAULT_WEBHOOK_JSON_PAYLOAD).toString( + 'base64' + ), }, }, webpush: { diff --git a/server/lib/settings/migrations/0009_migrate_webhook_payload_encoding.ts b/server/lib/settings/migrations/0009_migrate_webhook_payload_encoding.ts new file mode 100644 index 0000000000..f97e09cbff --- /dev/null +++ b/server/lib/settings/migrations/0009_migrate_webhook_payload_encoding.ts @@ -0,0 +1,41 @@ +import type { AllSettings } from '@server/lib/settings'; + +// Webhook payloads used to be stored as base64(JSON.stringify(template)); they +// are now stored as base64(template). Unwrap the JSON envelope on existing +// settings so the stored value is the raw (Liquid) template string. +const migrateWebhookPayloadEncoding = (settings: any): AllSettings => { + if ( + Array.isArray(settings.migrations) && + settings.migrations.includes('0009_migrate_webhook_payload_encoding') + ) { + return settings; + } + + const options = settings.notifications?.agents?.webhook?.options; + + if (options && typeof options.jsonPayload === 'string' && options.jsonPayload) { + try { + const decoded = Buffer.from(options.jsonPayload, 'base64').toString( + 'utf8' + ); + const unwrapped = JSON.parse(decoded); + + // Only the old format decodes to a JSON string literal; the new format is + // a raw template that isn't valid JSON, so JSON.parse throws and we skip. + if (typeof unwrapped === 'string') { + options.jsonPayload = Buffer.from(unwrapped).toString('base64'); + } + } catch { + // Already stored as a raw template (or not decodable) — leave as-is. + } + } + + if (!Array.isArray(settings.migrations)) { + settings.migrations = []; + } + settings.migrations.push('0009_migrate_webhook_payload_encoding'); + + return settings; +}; + +export default migrateWebhookPayloadEncoding; diff --git a/server/routes/settings/notifications.ts b/server/routes/settings/notifications.ts index 94b1f82b43..3e1fa64a9e 100644 --- a/server/routes/settings/notifications.ts +++ b/server/routes/settings/notifications.ts @@ -284,11 +284,10 @@ notificationRoutes.get('/webhook', (_req, res) => { types: webhookSettings.types, options: { ...webhookSettings.options, - jsonPayload: JSON.parse( - Buffer.from(webhookSettings.options.jsonPayload, 'base64').toString( - 'utf8' - ) - ), + jsonPayload: Buffer.from( + webhookSettings.options.jsonPayload, + 'base64' + ).toString('utf8'), customHeaders: webhookSettings.options.customHeaders ?? [], supportVariables: webhookSettings.options.supportVariables ?? false, }, @@ -305,9 +304,9 @@ notificationRoutes.post('/webhook', async (req, res, next) => { embedPoster: req.body.embedPoster, types: req.body.types, options: { - jsonPayload: Buffer.from( - JSON.stringify(req.body.options.jsonPayload) - ).toString('base64'), + jsonPayload: Buffer.from(req.body.options.jsonPayload).toString( + 'base64' + ), webhookUrl: req.body.options.webhookUrl, authHeader: req.body.options.authHeader, customHeaders: req.body.options.customHeaders ?? [], @@ -336,9 +335,9 @@ notificationRoutes.post('/webhook/test', async (req, res, next) => { embedPoster: req.body.embedPoster, types: req.body.types, options: { - jsonPayload: Buffer.from( - JSON.stringify(req.body.options.jsonPayload) - ).toString('base64'), + jsonPayload: Buffer.from(req.body.options.jsonPayload).toString( + 'base64' + ), webhookUrl: req.body.options.webhookUrl, authHeader: req.body.options.authHeader, customHeaders: req.body.options.customHeaders ?? [], From 3847a3d91ad3ec27be74112d4a19fa85c873c8be Mon Sep 17 00:00:00 2001 From: mmenanno <1358708+mmenanno@users.noreply.github.com> Date: Sun, 5 Jul 2026 16:04:08 -0400 Subject: [PATCH 3/4] docs(webhook): add liquid language to fenced code blocks Adds a language identifier to the two bare fenced code blocks in the webhook notification guide to satisfy markdownlint MD040, and registers liquid as an additional Prism language so Docusaurus highlights them without emitting an unknown-language warning. --- docs/using-seerr/notifications/webhook.md | 4 ++-- gen-docs/docusaurus.config.ts | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/using-seerr/notifications/webhook.md b/docs/using-seerr/notifications/webhook.md index 235f275f6d..db7bcc18f2 100644 --- a/docs/using-seerr/notifications/webhook.md +++ b/docs/using-seerr/notifications/webhook.md @@ -51,13 +51,13 @@ Payloads and webhook URLs are rendered with [LiquidJS](https://liquidjs.com/), s Render a section only when the relevant object is present, falling back to `null` otherwise: -``` +```liquid "media": {% if media %}{ "tmdbId": "{{ media_tmdbid }}" }{% else %}null{% endif %} ``` You can also branch on a value: -``` +```liquid {% if media_status == "AVAILABLE" %}...{% else %}...{% endif %} ``` diff --git a/gen-docs/docusaurus.config.ts b/gen-docs/docusaurus.config.ts index d2a6c07155..bee997b060 100644 --- a/gen-docs/docusaurus.config.ts +++ b/gen-docs/docusaurus.config.ts @@ -193,6 +193,7 @@ const config: Config = { 'nix', 'nginx', 'batch', + 'liquid', ], }, } satisfies Preset.ThemeConfig, From 4483ec829bb9554f28b16b7a7ecef21308380585 Mon Sep 17 00:00:00 2001 From: mmenanno <1358708+mmenanno@users.noreply.github.com> Date: Thu, 6 Aug 2026 01:18:15 -0400 Subject: [PATCH 4/4] refactor(notifications): address webhook templating review feedback - Make TemplateEngine.render async via parseAndRender, awaited in the already-async WebhookAgent.send - Test WebhookAgent.send outcomes (rendered payload posted, render/parse failures, URL template failure, post failure) with mocked axios instead of exercising LiquidJS engine behavior - Trim templateEngine tests to our own code: buildContext mapping and the circular-safe json filter - Clarify the 0009 settings migration comment with the storage-format change that makes it necessary - Link the intro-to-Liquid tutorial from the webhook docs --- docs/using-seerr/notifications/webhook.md | 2 +- .../lib/notifications/agents/webhook.test.ts | 114 ++++++++++++++++++ server/lib/notifications/agents/webhook.ts | 8 +- .../lib/notifications/templateEngine.test.ts | 56 +-------- server/lib/notifications/templateEngine.ts | 8 +- .../0009_migrate_webhook_payload_encoding.ts | 14 ++- 6 files changed, 135 insertions(+), 67 deletions(-) create mode 100644 server/lib/notifications/agents/webhook.test.ts diff --git a/docs/using-seerr/notifications/webhook.md b/docs/using-seerr/notifications/webhook.md index db7bcc18f2..b08ea3d4a2 100644 --- a/docs/using-seerr/notifications/webhook.md +++ b/docs/using-seerr/notifications/webhook.md @@ -39,7 +39,7 @@ You cannot configure both the **Authorization Header** field and a custom `Autho ### JSON Payload -Customize the JSON payload to suit your needs. The payload is a [Liquid](https://liquidjs.com/) template that is rendered when a notification is triggered and then sent as the request body. Seerr exposes a set of [template variables](#template-variables), plus the full `media`, `request`, `issue`, and `comment` objects, for use in [conditionals and filters](#template-syntax). +Customize the JSON payload to suit your needs. The payload is a [Liquid](https://liquidjs.com/) template that is rendered when a notification is triggered and then sent as the request body. Seerr exposes a set of [template variables](#template-variables), plus the full `media`, `request`, `issue`, and `comment` objects, for use in [conditionals and filters](#template-syntax). If you are new to Liquid, see the [introduction to Liquid tutorial](https://liquidjs.com/tutorials/intro-to-liquid.html). The template only needs to _render_ to valid JSON — it does not need to be valid JSON itself (for example, optional sections can be wrapped in `{% if %}` blocks). Use the **Test** button to confirm your template renders correctly; template errors are also written to the server logs. diff --git a/server/lib/notifications/agents/webhook.test.ts b/server/lib/notifications/agents/webhook.test.ts new file mode 100644 index 0000000000..e08d3e8bde --- /dev/null +++ b/server/lib/notifications/agents/webhook.test.ts @@ -0,0 +1,114 @@ +import { Notification } from '@server/lib/notifications'; +import WebhookAgent from '@server/lib/notifications/agents/webhook'; +import type { NotificationAgentWebhook } from '@server/lib/settings'; +import axios from 'axios'; +import assert from 'node:assert/strict'; +import { describe, it } from 'node:test'; +import type { NotificationPayload } from './agent'; + +const encode = (template: string) => Buffer.from(template).toString('base64'); + +const makePayload = ( + overrides: Partial = {} +): NotificationPayload => + ({ + subject: 'Test Subject', + notifySystem: true, + notifyAdmin: false, + ...overrides, + }) as NotificationPayload; + +const makeSettings = ( + overrides: Partial = {} +): NotificationAgentWebhook => ({ + enabled: true, + embedPoster: false, + types: Notification.TEST_NOTIFICATION, + options: { + webhookUrl: 'https://example.com/webhook', + jsonPayload: encode('{ "subject": {{ subject | json }} }'), + ...overrides, + }, +}); + +describe('WebhookAgent', () => { + describe('send', () => { + it('returns true and posts the rendered payload', async (t) => { + const post = t.mock.method(axios, 'post', async () => ({})); + + const agent = new WebhookAgent(makeSettings()); + const result = await agent.send( + Notification.TEST_NOTIFICATION, + makePayload({ subject: 'The "Best" Movie' }) + ); + + assert.equal(result, true); + assert.equal(post.mock.callCount(), 1); + const [url, body] = post.mock.calls[0].arguments; + assert.equal(url, 'https://example.com/webhook'); + assert.deepEqual(body, { subject: 'The "Best" Movie' }); + }); + + it('returns false when the payload template fails to render', async (t) => { + const post = t.mock.method(axios, 'post', async () => ({})); + + const agent = new WebhookAgent( + makeSettings({ jsonPayload: encode('{% invalid %}') }) + ); + const result = await agent.send( + Notification.TEST_NOTIFICATION, + makePayload() + ); + + assert.equal(result, false); + assert.equal(post.mock.callCount(), 0); + }); + + it('returns false when the rendered payload is not valid JSON', async (t) => { + const post = t.mock.method(axios, 'post', async () => ({})); + + const agent = new WebhookAgent( + makeSettings({ jsonPayload: encode('{ not json }') }) + ); + const result = await agent.send( + Notification.TEST_NOTIFICATION, + makePayload() + ); + + assert.equal(result, false); + assert.equal(post.mock.callCount(), 0); + }); + + it('returns false when the webhook URL template fails to render', async (t) => { + const post = t.mock.method(axios, 'post', async () => ({})); + + const agent = new WebhookAgent( + makeSettings({ + webhookUrl: 'https://example.com/{% invalid %}', + supportVariables: true, + }) + ); + const result = await agent.send( + Notification.TEST_NOTIFICATION, + makePayload() + ); + + assert.equal(result, false); + assert.equal(post.mock.callCount(), 0); + }); + + it('returns false when posting to the webhook URL fails', async (t) => { + t.mock.method(axios, 'post', async () => { + throw new Error('connect ECONNREFUSED'); + }); + + const agent = new WebhookAgent(makeSettings()); + const result = await agent.send( + Notification.TEST_NOTIFICATION, + makePayload() + ); + + assert.equal(result, false); + }); + }); +}); diff --git a/server/lib/notifications/agents/webhook.ts b/server/lib/notifications/agents/webhook.ts index 98ab7487d8..b9401dd6e2 100644 --- a/server/lib/notifications/agents/webhook.ts +++ b/server/lib/notifications/agents/webhook.ts @@ -21,12 +21,12 @@ class WebhookAgent return settings.notifications.agents.webhook; } - private buildPayload(type: Notification, payload: NotificationPayload) { + private async buildPayload(type: Notification, payload: NotificationPayload) { const template = Buffer.from( this.getSettings().options.jsonPayload, 'base64' ).toString('utf8'); - const rendered = TemplateEngine.render(template, payload, type); + const rendered = await TemplateEngine.render(template, payload, type); return JSON.parse(rendered); } @@ -64,7 +64,7 @@ class WebhookAgent if (settings.options.supportVariables) { try { - webhookUrl = TemplateEngine.render(webhookUrl, payload, type); + webhookUrl = await TemplateEngine.render(webhookUrl, payload, type); } catch (error) { logger.error('Failed to render webhook URL template', { label: 'Notifications', @@ -80,7 +80,7 @@ class WebhookAgent let body: unknown; try { - body = this.buildPayload(type, payload); + body = await this.buildPayload(type, payload); } catch (error) { logger.error('Failed to render webhook payload template', { label: 'Notifications', diff --git a/server/lib/notifications/templateEngine.test.ts b/server/lib/notifications/templateEngine.test.ts index 811bbf1d18..b824de267c 100644 --- a/server/lib/notifications/templateEngine.test.ts +++ b/server/lib/notifications/templateEngine.test.ts @@ -18,63 +18,11 @@ const makePayload = ( describe('TemplateEngine', () => { describe('render', () => { - it('escapes free-text values piped through the json filter', () => { - const payload = makePayload({ - subject: 'The "Best" Movie', - message: 'line one\nline two with a backslash \\', - }); - - const rendered = TemplateEngine.render( - '{ "subject": {{ subject | json }}, "message": {{ message | json }} }', - payload, - Notification.TEST_NOTIFICATION - ); - - const parsed = JSON.parse(rendered); - assert.equal(parsed.subject, 'The "Best" Movie'); - assert.equal(parsed.message, 'line one\nline two with a backslash \\'); - }); - - it('renders optional sections as null when the entity is absent', () => { - const template = - '{ "media": {% if media %}{ "tmdbId": "{{ media_tmdbid }}" }{% else %}null{% endif %} }'; - - const absent = JSON.parse( - TemplateEngine.render( - template, - makePayload(), - Notification.MEDIA_PENDING - ) - ); - assert.equal(absent.media, null); - - const present = JSON.parse( - TemplateEngine.render( - template, - makePayload({ media: { tmdbId: 42 } as never }), - Notification.MEDIA_PENDING - ) - ); - assert.deepEqual(present.media, { tmdbId: '42' }); - }); - - it('renders undefined variables as empty strings without throwing', () => { - const rendered = TemplateEngine.render( - '{ "known": {{ subject | json }}, "unknown": "{{ nonexistent_variable }}" }', - makePayload({ subject: 'Known Value' }), - Notification.TEST_NOTIFICATION - ); - - const parsed = JSON.parse(rendered); - assert.equal(parsed.known, 'Known Value'); - assert.equal(parsed.unknown, ''); - }); - - it('does not crash on circular references in the json filter', () => { + it('does not crash on circular references in the json filter', async () => { const circular: Record = {}; circular.self = circular; - const rendered = TemplateEngine.render( + const rendered = await TemplateEngine.render( '{{ extra | json }}', makePayload({ extra: circular as never }), Notification.TEST_NOTIFICATION diff --git a/server/lib/notifications/templateEngine.ts b/server/lib/notifications/templateEngine.ts index 604034bd59..f2824fa26f 100644 --- a/server/lib/notifications/templateEngine.ts +++ b/server/lib/notifications/templateEngine.ts @@ -148,13 +148,13 @@ export class TemplateEngine { * * @throws if the template contains invalid Liquid syntax */ - static render( + static async render( template: string, payload: NotificationPayload, type: Notification - ): string { + ): Promise { const context = this.buildContext(payload, type); - // parseAndRenderSync is typed `any`; coerce to honor the `string` return. - return String(this.getEngine().parseAndRenderSync(template, context)); + // parseAndRender is typed `Promise`; coerce to honor the `string` return. + return String(await this.getEngine().parseAndRender(template, context)); } } diff --git a/server/lib/settings/migrations/0009_migrate_webhook_payload_encoding.ts b/server/lib/settings/migrations/0009_migrate_webhook_payload_encoding.ts index f97e09cbff..6e48e5cd0e 100644 --- a/server/lib/settings/migrations/0009_migrate_webhook_payload_encoding.ts +++ b/server/lib/settings/migrations/0009_migrate_webhook_payload_encoding.ts @@ -1,8 +1,10 @@ import type { AllSettings } from '@server/lib/settings'; -// Webhook payloads used to be stored as base64(JSON.stringify(template)); they -// are now stored as base64(template). Unwrap the JSON envelope on existing -// settings so the stored value is the raw (Liquid) template string. +// The POST /settings/notifications/webhook route used to store the payload as +// base64(JSON.stringify(template)); it now stores base64(template). Unwrap the +// JSON envelope on existing settings so the stored value is the raw (Liquid) +// template string — otherwise the agent would decode a quoted JSON string +// literal and post a string body instead of the rendered object. const migrateWebhookPayloadEncoding = (settings: any): AllSettings => { if ( Array.isArray(settings.migrations) && @@ -13,7 +15,11 @@ const migrateWebhookPayloadEncoding = (settings: any): AllSettings => { const options = settings.notifications?.agents?.webhook?.options; - if (options && typeof options.jsonPayload === 'string' && options.jsonPayload) { + if ( + options && + typeof options.jsonPayload === 'string' && + options.jsonPayload + ) { try { const decoded = Buffer.from(options.jsonPayload, 'base64').toString( 'utf8'