Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion docs/content/docs/node/guides/extensibility/webhooks.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ queue.webhooks.delete(hook.id);
Deliveries fire from the **worker process** (where events originate). Verify the
signature on your endpoint by HMAC-ing the raw body with the shared secret.

Destinations are screened by an [SSRF
guard](/node/guides/operations/security#webhooks): loopback, private,
link-local, and cloud-metadata targets are rejected at registration and
re-checked before every attempt, and redirects are not followed. Set
`TASKITO_WEBHOOKS_ALLOW_PRIVATE=1` when developing against a local endpoint.

## Delivery log

`queue.webhooks.deliveries(id)` returns the most recent attempt-chains for a
Expand All @@ -52,7 +58,7 @@ for (const delivery of queue.webhooks.deliveries(hook.id)) {
| Field | Description |
|---|---|
| `id` / `webhookId` / `event` | Identifiers for the delivery and its subscription. |
| `status` | `"delivered"` (2xx) or `"dead"` (retries exhausted). |
| `status` | `"delivered"` (2xx), `"failed"` (refused — blocked URL or redirect), or `"dead"` (retries exhausted). |
| `ok` | `true` when `status` is `"delivered"`. |
| `attempts` | Number of HTTP attempts made. |
| `payload` | The JSON body that was POSTed. |
Expand Down
31 changes: 23 additions & 8 deletions docs/content/docs/node/guides/operations/security.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,27 @@ const expected = "sha256=" + createHmac("sha256", secret).update(rawBody).digest
// timing-safe compare against the x-taskito-signature header
```

The deliverer fetches the configured URL directly — there is **no built-in SSRF
(server-side request forgery) guard**. Only register webhook URLs you control.
Treat any user-provided URL as untrusted: enforce an egress allowlist, reject
private, loopback, link-local, and cloud-metadata ranges (including their IPv6
forms), re-resolve and re-validate the address immediately before delivery so
DNS rebinding and redirects can't swap in an internal target, and apply a strict
request timeout.
Delivery targets are checked against an **SSRF (server-side request forgery)
guard**. A URL must be `http`/`https` with a host, and the host must not be a
local-only name (`localhost`, `*.internal`, `*.local`, …) or resolve to a
loopback, unspecified, RFC1918, CGNAT, link-local (including
`169.254.169.254`), multicast, reserved, or IPv6 unique-local address. The URL
itself is checked when a subscription is registered and again before **every
delivery attempt**, and redirects are never followed — a `3xx` ends the
attempt-chain instead of walking past the guard. A blocked delivery is recorded
with status `failed` and is not retried.

```bash
TASKITO_WEBHOOKS_ALLOW_PRIVATE=1 # disable the guard for local development
```

The address check runs inside the resolver the socket dials with, so the address
connected to is the address that passed — a name rebound after registration, or
between the check and the connect, cannot slip through. The original hostname
still drives the Host header and TLS SNI. Resolution is bounded by the
subscription's `timeoutMs`; a resolver failure is a transient error and is
retried like any other. For user-supplied URLs, keep an egress allowlist in
front of the worker as defense in depth.

## Dashboard

Expand All @@ -70,7 +84,8 @@ can't be framed), and `Referrer-Policy: same-origin`.
- [ ] Storage reachable only from trusted services, on a private network.
- [ ] Enqueue behind your own authn/authz — never expose storage to clients.
- [ ] No secrets in task args; fetch them inside the handler.
- [ ] Webhook receivers verify the HMAC signature; webhook URLs are trusted.
- [ ] Webhook receivers verify the HMAC signature, and
`TASKITO_WEBHOOKS_ALLOW_PRIVATE` is unset in production.
- [ ] Dashboard auth enabled (`authEnabled: true` / `--auth`), and the port
bound to localhost or behind auth.
- [ ] Logs don't echo sensitive payloads (use `onEnqueue` to
Expand Down
16 changes: 14 additions & 2 deletions sdks/node/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,20 @@ export {
setLogLevel,
setLogSink,
} from "./utils";
export type { Delivery, Webhook, WebhookInput } from "./webhooks";
export { WebhookManager, WebhookValidationError } from "./webhooks";
export type {
Delivery,
SafeLookupOptions,
UrlSafetyOptions,
Webhook,
WebhookInput,
} from "./webhooks";
export {
assertSafeWebhookUrl,
createSafeLookup,
UnsafeWebhookUrlError,
WebhookManager,
WebhookValidationError,
} from "./webhooks";
export { Worker } from "./worker";
export type {
CanvasStep,
Expand Down
117 changes: 104 additions & 13 deletions sdks/node/src/webhooks/deliverer.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
import { createHmac, randomUUID } from "node:crypto";
import { request as httpRequest, type IncomingMessage } from "node:http";
import { request as httpsRequest } from "node:https";
import type { LookupFunction } from "node:net";
import type { EventName, OutcomeEvent } from "../events";
import { createLogger } from "../utils";
import { UnsafeWebhookUrlError } from "./errors";
import type { Delivery, Webhook } from "./types";
import { assertSafeWebhookUrl, createSafeLookup } from "./urlSafety";

const MAX_RECENT = 100;
const MAX_BACKOFF_MS = 30_000;
Expand Down Expand Up @@ -29,31 +34,62 @@ export class Deliverer {
headers["x-taskito-signature"] = `sha256=${signature}`;
}

// Resolution is bounded by the same budget as the request it precedes.
const safeLookup = createSafeLookup({ timeoutMs: webhook.timeoutMs });
const createdAt = Date.now();
let attempts = 0;
let responseCode: number | null = null;
let responseBody: string | null = null;
let error: string | undefined;
// Set when delivery is refused on policy grounds — retrying cannot help.
let nonRetryable = false;
while (attempts <= webhook.maxRetries) {
try {
// Re-checked per attempt so a subscription whose URL policy changed
// mid-chain stops here. Whether the *name* points somewhere private is
// settled by `safeLookup` at connect time, not guessed at up front.
assertSafeWebhookUrl(webhook.url);
} catch (cause) {
if (!(cause instanceof UnsafeWebhookUrlError)) {
throw cause;
}
responseCode = null;
responseBody = null;
error = cause.message;
nonRetryable = true;
break;
}
attempts += 1;
try {
const response = await fetch(webhook.url, {
method: "POST",
const response = await post(webhook.url, {
headers,
body,
signal: AbortSignal.timeout(webhook.timeoutMs),
timeoutMs: webhook.timeoutMs,
lookup: safeLookup,
});
responseCode = response.status;
responseBody = await readBody(response);
if (response.ok) {
responseBody = response.body;
if (response.status >= 200 && response.status < 300) {
error = undefined;
break;
}
// Redirects are never followed — one could point past the SSRF guard.
if (response.status >= 300 && response.status < 400) {
error = `redirect not followed (HTTP ${response.status})`;
nonRetryable = true;
break;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
error = `HTTP ${response.status}`;
} catch (cause) {
responseCode = null;
responseBody = null;
error = cause instanceof Error ? cause.message : String(cause);
// The address the socket resolved to is blocked; retrying re-resolves
// to the same place.
if (cause instanceof UnsafeWebhookUrlError) {
nonRetryable = true;
break;
}
}
if (attempts <= webhook.maxRetries) {
await sleep(backoff(attempts));
Expand All @@ -66,8 +102,9 @@ export class Deliverer {
id: randomUUID(),
webhookId: webhook.id,
event,
// The retry chain is inline, so a non-delivered outcome is `dead`.
status: ok ? "delivered" : "dead",
// The retry chain is inline: a refused delivery is `failed` (retrying
// cannot help), anything else that never landed is `dead`.
status: ok ? "delivered" : nonRetryable ? "failed" : "dead",
ok,
attempts,
payload: payloadRecord,
Expand All @@ -94,13 +131,67 @@ export class Deliverer {
}
}

interface PostOptions {
headers: Record<string, string>;
body: string;
timeoutMs: number;
/** Resolver for the destination; rejects addresses the guard blocks. */
lookup: LookupFunction;
}

interface PostResult {
status: number;
body: string | null;
}

/**
* POST `body` to `url` over `node:http(s)` rather than `fetch`.
*
* The core HTTP client takes a `lookup`, which is what lets the SSRF guard
* decide the address the socket dials, and it never follows redirects, so a
* `3xx` surfaces as itself on every runtime.
*/
function post(url: string, options: PostOptions): Promise<PostResult> {
return new Promise((resolve, reject) => {
const target = new URL(url);
const send = target.protocol === "https:" ? httpsRequest : httpRequest;
const request = send(
target,
{
method: "POST",
headers: {
...options.headers,
"content-length": Buffer.byteLength(options.body).toString(),
},
lookup: options.lookup,
},
(response) => {
readBody(response).then((text) =>
resolve({ status: response.statusCode ?? 0, body: text }),
);
},
);
request.setTimeout(options.timeoutMs, () => {
request.destroy(new Error(`timed out after ${options.timeoutMs}ms`));
});
request.on("error", reject);
request.end(options.body);
});
}

/** Read a response body for the delivery log, truncated; `null` if unreadable. */
async function readBody(response: Response): Promise<string | null> {
try {
return (await response.text()).slice(0, MAX_RESPONSE_BODY_CHARS);
} catch {
return null;
}
function readBody(response: IncomingMessage): Promise<string | null> {
return new Promise((resolve) => {
let text = "";
response.setEncoding("utf8");
response.on("data", (chunk: string) => {
if (text.length < MAX_RESPONSE_BODY_CHARS) {
text += chunk;
}
});
response.on("end", () => resolve(text.slice(0, MAX_RESPONSE_BODY_CHARS)));
response.on("error", () => resolve(null));
});
}

/** Strip credentials and query string from a URL before logging it. */
Expand Down
11 changes: 11 additions & 0 deletions sdks/node/src/webhooks/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,14 @@ export class WebhookValidationError extends TaskitoError {
this.name = "WebhookValidationError";
}
}

/**
* Thrown when a webhook URL targets a destination we refuse to deliver to
* (SSRF guard). A validation error so the dashboard still answers 400.
*/
export class UnsafeWebhookUrlError extends WebhookValidationError {
constructor(message: string) {
super(message);
this.name = "UnsafeWebhookUrlError";
}
}
8 changes: 7 additions & 1 deletion sdks/node/src/webhooks/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
export { WebhookValidationError } from "./errors";
export { UnsafeWebhookUrlError, WebhookValidationError } from "./errors";
export { WebhookManager } from "./manager";
export type { Delivery, Webhook, WebhookInput } from "./types";
export {
assertSafeWebhookUrl,
createSafeLookup,
type SafeLookupOptions,
type UrlSafetyOptions,
} from "./urlSafety";
9 changes: 4 additions & 5 deletions sdks/node/src/webhooks/manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { type DeliveryFilters, DeliveryLog } from "./deliveryLog";
import { WebhookValidationError } from "./errors";
import { WebhookStore } from "./store";
import type { Delivery, Webhook, WebhookInput } from "./types";
import { assertSafeWebhookUrl } from "./urlSafety";

const ALL_EVENTS: EventName[] = ["job.completed", "job.retrying", "job.dead", "job.cancelled"];

Expand All @@ -19,11 +20,9 @@ function validateWebhook(webhook: Webhook): void {
if (!webhook.url) {
throw new WebhookValidationError("webhook url is required");
}
try {
new URL(webhook.url);
} catch {
throw new WebhookValidationError(`invalid webhook url: ${webhook.url}`);
}
// Registration is synchronous, so only the checks that need no DNS run here;
// named hosts are resolved and re-checked on every delivery attempt.
assertSafeWebhookUrl(webhook.url);
if (!Number.isInteger(webhook.maxRetries) || webhook.maxRetries < 0) {
throw new WebhookValidationError("webhook maxRetries must be a non-negative integer");
}
Expand Down
Loading