fix(workflows): wire a real, SSRF-guarded webhook action (was a silent no-op)
Second-review loose end: the `webhook` node type passed validation but had no
registered handler → engine dispatched to registry.getAction('webhook') →
undefined → every run silently skipped. An enabled webhook flow no-op'd.
Register a real `webhook` action (covers both the webhook node type and the
"Call a webhook" action). It POSTs the run context to config.url, guarded by
validateExternalUrl — the same NAT64/private-range SSRF protection the webhook
delivery worker uses (GHSA-wmjx-pc37-272r) — with no redirects and a timeout,
unless WEBHOOK_ALLOW_PRIVATE_URLS=true (local-dev opt-out). Missing URL /
rejected URL / network error record an observable skipped step, not a crash.
So the action is now implemented → it passes the enable guard legitimately.
Test covers dry-run, missing-url, and metadata-IP (169.254.169.254) rejection.
This commit is contained in:
@@ -415,6 +415,24 @@ describe('workflow engine', () => {
|
|||||||
expect(await _internal.resolveTemplateKey('zzznotype', 'promo_')).toBe('promo_default');
|
expect(await _internal.resolveTemplateKey('zzznotype', 'promo_')).toBe('promo_default');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('webhook action: dry-run, missing url, and SSRF-guarded private/metadata url', async () => {
|
||||||
|
const webhook = engine.registry.getAction('webhook');
|
||||||
|
expect(typeof webhook).toBe('function'); // registered — no longer a silent no-op
|
||||||
|
const ctx = (config, vars = {}) => ({
|
||||||
|
run: { id: 1, workflow_id: 1, version: 1, trigger_event: 't', entity_type: 'x', entity_id: 1 },
|
||||||
|
node: { config }, vars, db, logger: { warn() {} },
|
||||||
|
});
|
||||||
|
// Dry run never calls out.
|
||||||
|
expect(await webhook(ctx({ url: 'https://example.com/hook' }, { __dryRun: true })))
|
||||||
|
.toMatchObject({ dryRun: true, would: 'webhook' });
|
||||||
|
// No URL → observable skip, not a crash.
|
||||||
|
expect(await webhook(ctx({}))).toMatchObject({ skipped: true });
|
||||||
|
// SSRF: cloud-metadata / private target rejected before any request.
|
||||||
|
const r = await webhook(ctx({ url: 'http://169.254.169.254/latest/meta-data' }));
|
||||||
|
expect(r.skipped).toBe(true);
|
||||||
|
expect(r.reason).toMatch(/rejected/i);
|
||||||
|
});
|
||||||
|
|
||||||
test('isBuiltinFlowActive reflects the built-in ENABLED state (enabled-based mutex)', async () => {
|
test('isBuiltinFlowActive reflects the built-in ENABLED state (enabled-based mutex)', async () => {
|
||||||
const { seedBuiltinWorkflowsAtBoot } = require('../../src/services/_workflowSeedBoot');
|
const { seedBuiltinWorkflowsAtBoot } = require('../../src/services/_workflowSeedBoot');
|
||||||
await seedBuiltinWorkflowsAtBoot(db, { info() {}, warn() {} });
|
await seedBuiltinWorkflowsAtBoot(db, { info() {}, warn() {} });
|
||||||
|
|||||||
@@ -179,6 +179,47 @@ registry.registerAction('notify_pre_event', async (ctx) => {
|
|||||||
return res;
|
return res;
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Call an external webhook (the `webhook` node type + the "Call a webhook"
|
||||||
|
// action both resolve here). POSTs the run context to config.url. SSRF-guarded
|
||||||
|
// via validateExternalUrl — the same NAT64/private-range protection the webhook
|
||||||
|
// delivery worker uses — unless WEBHOOK_ALLOW_PRIVATE_URLS=true (local-dev
|
||||||
|
// opt-out). No redirects. Best-effort: a rejected URL / network error records an
|
||||||
|
// observable skipped step rather than throwing the run.
|
||||||
|
registry.registerAction('webhook', async (ctx) => {
|
||||||
|
const url = ctx.node.config?.url;
|
||||||
|
if (!url) return { skipped: true, reason: 'no webhook url configured' };
|
||||||
|
if (ctx.vars?.__dryRun) return { dryRun: true, would: 'webhook', url };
|
||||||
|
|
||||||
|
if (process.env.WEBHOOK_ALLOW_PRIVATE_URLS !== 'true') {
|
||||||
|
const { validateExternalUrl } = require('../../utils/networkValidation');
|
||||||
|
const check = validateExternalUrl(url);
|
||||||
|
if (!check.valid) return { skipped: true, reason: `url rejected: ${check.error}` };
|
||||||
|
}
|
||||||
|
|
||||||
|
const axios = require('axios');
|
||||||
|
const body = {
|
||||||
|
workflow: { id: ctx.run.workflow_id, version: ctx.run.version },
|
||||||
|
run: {
|
||||||
|
id: ctx.run.id,
|
||||||
|
trigger_event: ctx.run.trigger_event,
|
||||||
|
entity_type: ctx.run.entity_type,
|
||||||
|
entity_id: ctx.run.entity_id,
|
||||||
|
},
|
||||||
|
vars: ctx.vars || {},
|
||||||
|
};
|
||||||
|
try {
|
||||||
|
const res = await axios.post(url, body, {
|
||||||
|
headers: { 'Content-Type': 'application/json', 'User-Agent': 'PicPeak-Workflows/1.0', 'X-PicPeak-Event': ctx.run.trigger_event || '' },
|
||||||
|
timeout: parseInt(process.env.WEBHOOK_HTTP_TIMEOUT_MS || '10000', 10),
|
||||||
|
maxRedirects: 0, // no redirects — SSRF + receivers should give a final URL
|
||||||
|
validateStatus: () => true,
|
||||||
|
});
|
||||||
|
return { webhook_posted: url, status: res.status };
|
||||||
|
} catch (err) {
|
||||||
|
return { skipped: true, reason: `webhook request failed: ${err.message}` };
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
// Create/prepare-document actions — registered so flows referencing them are
|
// Create/prepare-document actions — registered so flows referencing them are
|
||||||
// valid; service wiring is a follow-up. Records a skipped step (observable).
|
// valid; service wiring is a follow-up. Records a skipped step (observable).
|
||||||
for (const key of DOCUMENT_ACTIONS) {
|
for (const key of DOCUMENT_ACTIONS) {
|
||||||
|
|||||||
Reference in New Issue
Block a user