mirror of
https://github.com/wekan/wekan.git
synced 2026-11-07 11:02:33 -05:00
Lauri Ojansivu
b01c233269
The activity hook delivered webhooks through Meteor.call('outgoingWebhooks'),
and that method trusted a call without this.connection as the server's own.
Meteor gives a server-side Meteor.call the userId AND connection of the method
it runs in (ddp-server applyAsync), and a card moved from a browser runs in
that browser's method. So the server's own act-moveCard delivery arrived as a
client forging one: it was recorded as ssrf.webhook-forge (high) with the
mover's userId, and blockOnSecurityEvent set loginDisabled.
- deliverActivityWebhook() in server/notifications/outgoing.js is the server's
delivery: a function, not a method, so no client reaches it and no inherited
context changes it. It reads the integration again (enabled) and names the
activity's actor. server/models/activities.js calls it through safeDeliver.
- The outgoingWebhooks method treats every call as a client's: only the
card-opened notification, rebuilt from the card, as before for clients.
- Server deliveries no longer need the actor to be a member of the hook's
board, which also skipped a global webhook ('_global' is no board) and an
activity with no method behind it (REST, timers).
- securityLog.record() now passes the catalog key and blocksAccount to the
block decision. The stored row carries neither, so every 'high' blocked row
disabled whoever it named: a hook refused for its own internal address
disabled the mover too, and ssrf.redirect/ssrf.attachment's
blocksAccount: false never applied. ssrf.webhook is now blocksAccount:
false - the person whose move sent it did not write the hook's URL.
tests/hookBleedServerDelivery.test.cjs: the move delivered with nothing
logged, in an inherited context too; a forged call still refused, logged and
blocking; the card-opened notification; failing and internal hooks blocking
nobody, through record() itself; and a whole-tree scan that no server code
calls a this.connection-gated method from inside a client's method without
clearing the invocation. tests/playwright/specs/hook-bleed.e2e.js moves a
card through the Move dialog with a webhook enabled.
Thanks to do4me and xet7 !
Fixes #6782.
47 lines
1.7 KiB
JavaScript
47 lines
1.7 KiB
JavaScript
/**
|
|
* server/lib/webhookGuard.js — non-blocking, error-isolated webhook delivery
|
|
*
|
|
* Outgoing webhooks are fired from the `Activities.after.insert` hook. If the
|
|
* delivery throws (network error, timeout, slow/unreachable endpoint, invalid
|
|
* response), that exception must NOT propagate into the activity insert or the
|
|
* originating operation (e.g. adding/removing a card member). Otherwise the
|
|
* member update is aborted / rolled back and never persists.
|
|
*
|
|
* `safeDeliver` runs the supplied delivery function and ALWAYS resolves — it
|
|
* never rejects. On success it resolves `{ ok: true }`; on any thrown error
|
|
* (sync or async) it logs and resolves `{ ok: false, error }`.
|
|
*
|
|
* This is a pure, dependency-free helper so it can be unit tested without
|
|
* Meteor.
|
|
*
|
|
* Usage:
|
|
* import { safeDeliver } from '/server/lib/webhookGuard';
|
|
* safeDeliver(() => deliverActivityWebhook(integration, description, params, actorId));
|
|
*/
|
|
|
|
/**
|
|
* Run `deliverFn` and resolve regardless of outcome (never rejects).
|
|
*
|
|
* @param {Function} deliverFn - function performing the webhook delivery; may
|
|
* be synchronous or return a Promise.
|
|
* @returns {Promise<{ok: boolean, error?: Error}>}
|
|
*/
|
|
export async function safeDeliver(deliverFn) {
|
|
try {
|
|
if (typeof deliverFn !== 'function') {
|
|
throw new TypeError('safeDeliver: deliverFn must be a function');
|
|
}
|
|
await deliverFn();
|
|
return { ok: true };
|
|
} catch (error) {
|
|
// Swallow + log: a failing webhook must never abort the caller.
|
|
try {
|
|
// eslint-disable-next-line no-console
|
|
console.error('[webhookGuard] outgoing webhook delivery failed:', error && error.message ? error.message : error);
|
|
} catch (_logErr) {
|
|
// ignore logging failures
|
|
}
|
|
return { ok: false, error };
|
|
}
|
|
}
|