fix: secure webhook config endpoint and validate config on Ombi enable/test
Build and Push Docker Image / build (push) Successful in 1m4s
Docs Check / Markdown lint (push) Successful in 1m49s
Licence Check / Licence compatibility and copyright header verification (push) Failing after 2m22s
CI / Security audit (push) Successful in 2m44s
CI / Swagger Validation & Coverage (push) Successful in 2m59s
Docs Check / Mermaid diagram parse check (push) Successful in 3m11s
CI / Tests & coverage (push) Successful in 3m27s

- Add requireAuth to GET /api/webhook/config to enforce authentication
- Add SOFARR_BASE_URL and SOFARR_WEBHOOK_SECRET validation to POST /api/ombi/webhook/enable and /test
- Return 400 with descriptive errors when webhook config is missing on Ombi routes
- Clean up test environment in webhook.test.js afterEach
- Add regression tests for all new validation logic
- Update CHANGELOG.md with security fixes
This commit is contained in:
2026-05-22 09:50:30 +01:00
parent f1e0a77fad
commit dbf45ec31d
6 changed files with 392 additions and 22 deletions
+3
View File
@@ -13,6 +13,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- **Webhook configuration check** — Ombi webhook status now correctly checks for `SOFARR_BASE_URL` and `SOFARR_WEBHOOK_SECRET` environment variables. The webhook displays as disabled when either is missing, matching the existing *ARR webhook behavior.
- **Common webhook config endpoint** — Added `GET /api/webhook/config` endpoint that returns whether both `SOFARR_BASE_URL` and `SOFARR_WEBHOOK_SECRET` are configured, along with a list of missing items. Used by the client to determine webhook enablement status across all webhook types.
- **Client-side webhook status** — Updated `fetchWebhookStatus()` to call `/api/webhook/config` and use the result to determine if Sonarr and Radarr webhooks are enabled. Webhook status now depends on both the service-specific webhook being configured AND the Sofarr webhook config being valid.
- **Webhook config endpoint authentication** — Added `requireAuth` middleware to `GET /api/webhook/config` to enforce authentication, matching the OpenAPI contract and preventing unauthenticated information disclosure.
- **Ombi webhook enable/test config validation** — `POST /api/ombi/webhook/enable` and `POST /api/ombi/webhook/test` now validate `SOFARR_BASE_URL` and `SOFARR_WEBHOOK_SECRET` before making outbound Ombi API calls, returning `400` with descriptive errors when either is missing. Previously these routes would construct malformed URLs (`undefined/api/webhook/ombi`) if the environment variables were unset.
- **Test environment cleanup** — `tests/integration/webhook.test.js` `afterEach` now cleans up `EMBY_URL`, `SOFARR_BASE_URL`, `SONARR_INSTANCES`, and `RADARR_INSTANCES` to ensure hermetic test isolation.
---
+16 -4
View File
@@ -137,7 +137,19 @@ export async function fetchWebhookStatus() {
try {
// Fetch metrics in parallel
const metricsPromise = fetchWebhookMetrics();
// Fetch webhook configuration status (checks SOFARR_BASE_URL and SOFARR_WEBHOOK_SECRET)
let webhookConfigValid = false;
try {
const configRes = await fetch('/api/webhook/config');
if (configRes.ok) {
const configData = await configRes.json();
webhookConfigValid = configData.valid || false;
}
} catch (err) {
// Config endpoint not available, assume invalid
}
// Fetch Sonarr notifications
let sonarrEnabled = false;
let sonarrTriggers = { onGrab: false, onDownload: false, onImport: false, onUpgrade: false };
@@ -146,7 +158,7 @@ export async function fetchWebhookStatus() {
if (sonarrRes.ok) {
const sonarrData = await sonarrRes.json();
const sonarrSofarr = sonarrData.find(n => n.name === 'Sofarr');
sonarrEnabled = !!sonarrSofarr;
sonarrEnabled = webhookConfigValid && !!sonarrSofarr;
if (sonarrSofarr) {
sonarrTriggers = {
onGrab: sonarrSofarr.onGrab,
@@ -159,7 +171,7 @@ export async function fetchWebhookStatus() {
} catch (err) {
// Sonarr not configured
}
// Fetch Radarr notifications
let radarrEnabled = false;
let radarrTriggers = { onGrab: false, onDownload: false, onImport: false, onUpgrade: false };
@@ -168,7 +180,7 @@ export async function fetchWebhookStatus() {
if (radarrRes.ok) {
const radarrData = await radarrRes.json();
const radarrSofarr = radarrData.find(n => n.name === 'Sofarr');
radarrEnabled = !!radarrSofarr;
radarrEnabled = webhookConfigValid && !!radarrSofarr;
if (radarrSofarr) {
radarrTriggers = {
onGrab: radarrSofarr.onGrab,
+47 -7
View File
@@ -2,7 +2,7 @@
const express = require('express');
const { logToFile } = require('../utils/logger');
const cache = require('../utils/cache');
const { getOmbiInstances } = require('../utils/config');
const { getOmbiInstances, getWebhookSecret, getSofarrBaseUrl } = require('../utils/config');
const requireAuth = require('../middleware/requireAuth');
const { extractRequestedUser } = require('../utils/ombiHelpers');
@@ -153,13 +153,23 @@ router.get('/requests', requireAuth, async (req, res) => {
*/
router.post('/webhook/enable', requireAuth, async (req, res) => {
try {
const sofarrBaseUrl = getSofarrBaseUrl();
const webhookSecret = getWebhookSecret();
if (!sofarrBaseUrl) {
return res.status(400).json({ error: 'SOFARR_BASE_URL not configured' });
}
if (!webhookSecret) {
return res.status(400).json({ error: 'SOFARR_WEBHOOK_SECRET not configured' });
}
const ombiInstances = getOmbiInstances();
if (ombiInstances.length === 0) {
return res.status(400).json({ error: 'Ombi not configured' });
}
const ombiInst = ombiInstances[0];
const webhookUrl = `${process.env.SOFARR_BASE_URL}/api/webhook/ombi`;
const webhookUrl = `${sofarrBaseUrl}/api/webhook/ombi`;
// Call Ombi API to register webhook
const axios = require('axios');
@@ -261,11 +271,31 @@ router.post('/webhook/enable', requireAuth, async (req, res) => {
*/
router.get('/webhook/status', requireAuth, async (req, res) => {
try {
const sofarrBaseUrl = getSofarrBaseUrl();
const webhookSecret = getWebhookSecret();
// Webhooks require SOFARR_BASE_URL and SOFARR_WEBHOOK_SECRET to be configured
if (!sofarrBaseUrl || !webhookSecret) {
return res.json({
enabled: false,
webhookUrl: null,
applicationToken: null,
triggers: {
requestAvailable: false,
requestApproved: false,
requestDeclined: false,
requestPending: false,
requestProcessing: false
},
stats: null
});
}
const ombiInstances = getOmbiInstances();
if (ombiInstances.length === 0) {
return res.json({
enabled: false,
webhookUrl: null,
return res.json({
enabled: false,
webhookUrl: null,
applicationToken: null,
triggers: {
requestAvailable: false,
@@ -360,13 +390,23 @@ router.get('/webhook/status', requireAuth, async (req, res) => {
*/
router.post('/webhook/test', requireAuth, async (req, res) => {
try {
const sofarrBaseUrl = getSofarrBaseUrl();
const webhookSecret = getWebhookSecret();
if (!sofarrBaseUrl) {
return res.status(400).json({ error: 'SOFARR_BASE_URL not configured' });
}
if (!webhookSecret) {
return res.status(400).json({ error: 'SOFARR_WEBHOOK_SECRET not configured' });
}
const ombiInstances = getOmbiInstances();
if (ombiInstances.length === 0) {
return res.status(400).json({ error: 'Ombi not configured' });
}
const ombiInst = ombiInstances[0];
const webhookUrl = `${process.env.SOFARR_BASE_URL}/api/webhook/ombi`;
const webhookUrl = `${sofarrBaseUrl}/api/webhook/ombi`;
// Simulate a test webhook event
const axios = require('axios');
@@ -379,7 +419,7 @@ router.post('/webhook/test', requireAuth, async (req, res) => {
requestStatus: 'Pending'
}, {
headers: {
'X-Sofarr-Webhook-Secret': process.env.SOFARR_WEBHOOK_SECRET,
'X-Sofarr-Webhook-Secret': webhookSecret,
'Content-Type': 'application/json'
}
});
+58 -1
View File
@@ -2,14 +2,71 @@
const express = require('express');
const rateLimit = require('express-rate-limit');
const { logToFile } = require('../utils/logger');
const { getWebhookSecret, getSonarrInstances, getRadarrInstances, getOmbiInstances } = require('../utils/config');
const { getWebhookSecret, getSonarrInstances, getRadarrInstances, getOmbiInstances, getSofarrBaseUrl } = require('../utils/config');
const cache = require('../utils/cache');
const arrRetrieverRegistry = require('../utils/arrRetrievers');
const { pollAllServices, POLL_INTERVAL, POLLING_ENABLED } = require('../utils/poller');
const { extractRequestedUser } = require('../utils/ombiHelpers');
const requireAuth = require('../middleware/requireAuth');
const router = express.Router();
/**
* @openapi
* /api/webhook/config:
* get:
* tags: [Webhook]
* summary: Get webhook configuration status
* description: |
* Returns whether the required webhook configuration (SOFARR_BASE_URL and SOFARR_WEBHOOK_SECRET)
* is properly configured. Used by the webhooks panel to determine if webhooks can be enabled.
*
* **Authentication:** Requires valid `emby_user` cookie.
* security:
* - CookieAuth: []
* responses:
* '200':
* description: Webhook configuration status
* content:
* application/json:
* schema:
* type: object
* properties:
* valid:
* type: boolean
* description: true if both SOFARR_BASE_URL and SOFARR_WEBHOOK_SECRET are configured
* example: true
* missing:
* type: array
* items:
* type: string
* description: List of missing configuration items
* example: []
* '401':
* description: Unauthorized
* content:
* application/json:
* schema:
* $ref: '#/components/schemas/ErrorResponse'
*/
router.get('/config', requireAuth, (req, res) => {
const sofarrBaseUrl = getSofarrBaseUrl();
const webhookSecret = getWebhookSecret();
const missing = [];
if (!sofarrBaseUrl) {
missing.push('SOFARR_BASE_URL');
}
if (!webhookSecret) {
missing.push('SOFARR_WEBHOOK_SECRET');
}
res.json({
valid: missing.length === 0,
missing
});
});
// Dedicated rate limiter for webhook endpoints — stricter than the global API limiter.
// Sonarr/Radarr send at most one event per action; 60/min per IP is generous.
// In tests, SKIP_RATE_LIMIT=1 raises the ceiling to effectively unlimited.
+143 -10
View File
@@ -405,17 +405,90 @@ describe('GET /api/ombi/webhook/status', () => {
expect(res.body.error).toBe('Not authenticated');
});
it('returns disabled status when SOFARR_BASE_URL is missing', async () => {
delete process.env.SOFARR_BASE_URL;
app = createApp({ skipRateLimits: true });
const { cookies } = await authenticateUser(app, 'TestUser', false);
const res = await request(app)
.get('/api/ombi/webhook/status')
.set('Cookie', cookies)
.expect(200);
expect(res.body.enabled).toBe(false);
expect(res.body.webhookUrl).toBeNull();
expect(res.body.applicationToken).toBeNull();
expect(res.body.triggers).toEqual({
requestAvailable: false,
requestApproved: false,
requestDeclined: false,
requestPending: false,
requestProcessing: false
});
expect(res.body.stats).toBeNull();
});
it('returns disabled status when SOFARR_WEBHOOK_SECRET is missing', async () => {
delete process.env.SOFARR_WEBHOOK_SECRET;
app = createApp({ skipRateLimits: true });
const { cookies } = await authenticateUser(app, 'TestUser', false);
const res = await request(app)
.get('/api/ombi/webhook/status')
.set('Cookie', cookies)
.expect(200);
expect(res.body.enabled).toBe(false);
expect(res.body.webhookUrl).toBeNull();
expect(res.body.applicationToken).toBeNull();
expect(res.body.triggers).toEqual({
requestAvailable: false,
requestApproved: false,
requestDeclined: false,
requestPending: false,
requestProcessing: false
});
expect(res.body.stats).toBeNull();
});
it('returns disabled status when both SOFARR_BASE_URL and SOFARR_WEBHOOK_SECRET are missing', async () => {
delete process.env.SOFARR_BASE_URL;
delete process.env.SOFARR_WEBHOOK_SECRET;
app = createApp({ skipRateLimits: true });
const { cookies } = await authenticateUser(app, 'TestUser', false);
const res = await request(app)
.get('/api/ombi/webhook/status')
.set('Cookie', cookies)
.expect(200);
expect(res.body.enabled).toBe(false);
expect(res.body.webhookUrl).toBeNull();
expect(res.body.applicationToken).toBeNull();
expect(res.body.triggers).toEqual({
requestAvailable: false,
requestApproved: false,
requestDeclined: false,
requestPending: false,
requestProcessing: false
});
expect(res.body.stats).toBeNull();
});
it('returns disabled status when Ombi not configured', async () => {
process.env.OMBI_INSTANCES = JSON.stringify([]);
app = createApp({ skipRateLimits: true });
const { cookies } = await authenticateUser(app, 'TestUser', false);
const res = await request(app)
.get('/api/ombi/webhook/status')
.set('Cookie', cookies)
.expect(200);
expect(res.body.enabled).toBe(false);
expect(res.body.webhookUrl).toBeNull();
expect(res.body.applicationToken).toBeNull();
@@ -559,18 +632,48 @@ describe('POST /api/ombi/webhook/enable', () => {
expect(res.body.error).toBe('CSRF token missing');
});
it('returns 400 when Ombi not configured', async () => {
process.env.OMBI_INSTANCES = JSON.stringify([]);
it('returns 400 when SOFARR_BASE_URL is missing', async () => {
delete process.env.SOFARR_BASE_URL;
app = createApp({ skipRateLimits: true });
const { cookies, csrfToken } = await authenticateUser(app, 'TestUser', false);
const res = await request(app)
.post('/api/ombi/webhook/enable')
.set('Cookie', cookies)
.set('X-CSRF-Token', csrfToken)
.expect(400);
expect(res.body.error).toBe('SOFARR_BASE_URL not configured');
});
it('returns 400 when SOFARR_WEBHOOK_SECRET is missing', async () => {
delete process.env.SOFARR_WEBHOOK_SECRET;
app = createApp({ skipRateLimits: true });
const { cookies, csrfToken } = await authenticateUser(app, 'TestUser', false);
const res = await request(app)
.post('/api/ombi/webhook/enable')
.set('Cookie', cookies)
.set('X-CSRF-Token', csrfToken)
.expect(400);
expect(res.body.error).toBe('SOFARR_WEBHOOK_SECRET not configured');
});
it('returns 400 when Ombi not configured', async () => {
process.env.OMBI_INSTANCES = JSON.stringify([]);
app = createApp({ skipRateLimits: true });
const { cookies, csrfToken } = await authenticateUser(app, 'TestUser', false);
const res = await request(app)
.post('/api/ombi/webhook/enable')
.set('Cookie', cookies)
.set('X-CSRF-Token', csrfToken)
.expect(400);
expect(res.body.error).toBe('Ombi not configured');
});
@@ -627,18 +730,48 @@ describe('POST /api/ombi/webhook/test', () => {
expect(res.body.error).toBe('CSRF token missing');
});
it('returns 400 when Ombi not configured', async () => {
process.env.OMBI_INSTANCES = JSON.stringify([]);
it('returns 400 when SOFARR_BASE_URL is missing', async () => {
delete process.env.SOFARR_BASE_URL;
app = createApp({ skipRateLimits: true });
const { cookies, csrfToken } = await authenticateUser(app, 'TestUser', false);
const res = await request(app)
.post('/api/ombi/webhook/test')
.set('Cookie', cookies)
.set('X-CSRF-Token', csrfToken)
.expect(400);
expect(res.body.error).toBe('SOFARR_BASE_URL not configured');
});
it('returns 400 when SOFARR_WEBHOOK_SECRET is missing', async () => {
delete process.env.SOFARR_WEBHOOK_SECRET;
app = createApp({ skipRateLimits: true });
const { cookies, csrfToken } = await authenticateUser(app, 'TestUser', false);
const res = await request(app)
.post('/api/ombi/webhook/test')
.set('Cookie', cookies)
.set('X-CSRF-Token', csrfToken)
.expect(400);
expect(res.body.error).toBe('SOFARR_WEBHOOK_SECRET not configured');
});
it('returns 400 when Ombi not configured', async () => {
process.env.OMBI_INSTANCES = JSON.stringify([]);
app = createApp({ skipRateLimits: true });
const { cookies, csrfToken } = await authenticateUser(app, 'TestUser', false);
const res = await request(app)
.post('/api/ombi/webhook/test')
.set('Cookie', cookies)
.set('X-CSRF-Token', csrfToken)
.expect(400);
expect(res.body.error).toBe('Ombi not configured');
});
+125
View File
@@ -28,6 +28,18 @@ const require = createRequire(import.meta.url);
const cache = require('../../server/utils/cache.js');
const VALID_SECRET = 'test-webhook-secret-abc';
const EMBY_BASE = 'https://emby.test';
const EMBY_AUTH_BODY = {
AccessToken: 'test-emby-token-abc123',
User: { Id: 'user-id-001', Name: 'TestUser' }
};
const EMBY_USER_BODY = {
Id: 'user-id-001',
Name: 'TestUser',
Policy: { IsAdministrator: false }
};
// Minimal valid Sonarr Grab payload
const SONARR_GRAB = {
@@ -53,7 +65,31 @@ const SONARR_TEST = {
date: '2026-05-19T10:00:02.000Z'
};
function interceptSuccessfulLogin(userBody = EMBY_USER_BODY) {
nock(EMBY_BASE)
.post('/Users/authenticatebyname')
.reply(200, EMBY_AUTH_BODY);
nock(EMBY_BASE)
.get(/\/Users\//)
.reply(200, userBody);
}
async function authenticateUser(app, username = 'TestUser', isAdmin = false) {
const userBody = isAdmin ? { ...EMBY_USER_BODY, Policy: { IsAdministrator: true } } : EMBY_USER_BODY;
interceptSuccessfulLogin(userBody);
const res = await request(app)
.post('/api/auth/login')
.send({ username, password: 'password' });
const cookies = res.headers['set-cookie'];
const csrfToken = res.body.csrfToken;
return { cookies, csrfToken };
}
function makeApp() {
process.env.EMBY_URL = EMBY_BASE;
process.env.SOFARR_WEBHOOK_SECRET = VALID_SECRET;
process.env.SONARR_INSTANCES = JSON.stringify([
{ id: 'sonarr-1', name: 'Main Sonarr', url: 'https://sonarr.test', apiKey: 'sk' }
@@ -84,7 +120,11 @@ beforeEach(() => {
afterEach(() => {
nock.cleanAll();
delete process.env.EMBY_URL;
delete process.env.SOFARR_WEBHOOK_SECRET;
delete process.env.SOFARR_BASE_URL;
delete process.env.SONARR_INSTANCES;
delete process.env.RADARR_INSTANCES;
});
// ---------------------------------------------------------------------------
@@ -393,3 +433,88 @@ describe('Security — secret never leaks', () => {
expect(JSON.stringify(res.body)).not.toContain(VALID_SECRET);
});
});
// ---------------------------------------------------------------------------
// GET /api/webhook/config
// ---------------------------------------------------------------------------
describe('GET /api/webhook/config', () => {
it('returns 401 when not authenticated', async () => {
const app = makeApp();
const res = await request(app)
.get('/api/webhook/config')
.expect(401);
expect(res.body.error).toBe('Not authenticated');
});
it('returns valid: true when both SOFARR_BASE_URL and SOFARR_WEBHOOK_SECRET are configured', async () => {
process.env.EMBY_URL = EMBY_BASE;
process.env.SOFARR_BASE_URL = 'https://sofarr.test';
process.env.SOFARR_WEBHOOK_SECRET = VALID_SECRET;
const app = createApp({ skipRateLimits: true });
const { cookies } = await authenticateUser(app, 'TestUser', false);
const res = await request(app)
.get('/api/webhook/config')
.set('Cookie', cookies)
.expect(200);
expect(res.body.valid).toBe(true);
expect(res.body.missing).toEqual([]);
});
it('returns valid: false when SOFARR_BASE_URL is missing', async () => {
process.env.EMBY_URL = EMBY_BASE;
delete process.env.SOFARR_BASE_URL;
process.env.SOFARR_WEBHOOK_SECRET = VALID_SECRET;
const app = createApp({ skipRateLimits: true });
const { cookies } = await authenticateUser(app, 'TestUser', false);
const res = await request(app)
.get('/api/webhook/config')
.set('Cookie', cookies)
.expect(200);
expect(res.body.valid).toBe(false);
expect(res.body.missing).toEqual(['SOFARR_BASE_URL']);
});
it('returns valid: false when SOFARR_WEBHOOK_SECRET is missing', async () => {
process.env.EMBY_URL = EMBY_BASE;
process.env.SOFARR_BASE_URL = 'https://sofarr.test';
delete process.env.SOFARR_WEBHOOK_SECRET;
const app = createApp({ skipRateLimits: true });
const { cookies } = await authenticateUser(app, 'TestUser', false);
const res = await request(app)
.get('/api/webhook/config')
.set('Cookie', cookies)
.expect(200);
expect(res.body.valid).toBe(false);
expect(res.body.missing).toEqual(['SOFARR_WEBHOOK_SECRET']);
});
it('returns valid: false when both are missing', async () => {
process.env.EMBY_URL = EMBY_BASE;
delete process.env.SOFARR_BASE_URL;
delete process.env.SOFARR_WEBHOOK_SECRET;
const app = createApp({ skipRateLimits: true });
const { cookies } = await authenticateUser(app, 'TestUser', false);
const res = await request(app)
.get('/api/webhook/config')
.set('Cookie', cookies)
.expect(200);
expect(res.body.valid).toBe(false);
expect(res.body.missing).toContain('SOFARR_BASE_URL');
expect(res.body.missing).toContain('SOFARR_WEBHOOK_SECRET');
expect(res.body.missing).toHaveLength(2);
});
});