mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-26 09:52:11 +03:00
Webhooks hardcoded parseAndValidatePublicUrl, which blocks any RFC1918/loopback host — breaking self-hosted setups that legitimately point webhooks at internal services (n8n, Home Assistant, a LAN box). Provider URLs already had an opt-in (OMNIROUTE_ALLOW_PRIVATE_PROVIDER_URLS); webhooks now reuse it. - outboundUrlGuard: add parseAndValidateWebhookUrl — gates the private-host check on arePrivateProviderUrlsAllowed() (default OFF); protocol + embedded-credential checks stay unconditional. - swap all webhook call sites (create/update/test/validate-url + dispatcher x2) to it. Regression test: tests/unit/webhook-private-optin-3269.test.ts (RED before, GREEN after); existing webhook SSRF/dispatcher suites stay green (33/33).
This commit is contained in:
committed by
GitHub
parent
a19cfd4036
commit
a5d19bf4b9
@@ -44,6 +44,11 @@ Thanks to everyone whose work landed in v3.8.12:
|
||||
| [@leninejunior](https://github.com/leninejunior) | #3271 |
|
||||
| [@zhiru](https://github.com/zhiru) | #3274 |
|
||||
| [@diegosouzapw](https://github.com/diegosouzapw) | maintainer — #3256, #3263, #3270, #3275, #3277, #3278 |
|
||||
- **api/responses:** combo names without a slash (e.g. `paid-premium`, `n8n-text`) are no longer force-rewritten to `codex/<combo>` on `/v1/responses` — `resolveResponsesApiModel` now returns the request unchanged when the model resolves to a combo (regression from the v3.8.9 Codex WS→HTTP fallback) ([#3242](https://github.com/diegosouzapw/OmniRoute/pull/3242) — thanks @wilsonicdev; the same fix shipped via #3244, closing #3227 / #3233)
|
||||
- **sse/web-tools:** web-cookie providers (e.g. `ds-web`) that wrap tool calls as `<tool_call name="...">{json}</tool_call>` are now parsed correctly — the real tool name is read from the JSON body instead of the tag attribute, and the call is no longer silently dropped when `arguments` is absent ([#3260](https://github.com/diegosouzapw/OmniRoute/issues/3260))
|
||||
- **sse/groq:** non-reasoning Groq models (`llama-3.3-70b-versatile`, `llama-4-scout`) are now flagged `supportsReasoning: false`, so `reasoning_effort` / `output_config.effort` / `thinking` are stripped before dispatch instead of being forwarded and rejected with HTTP 400 — fixes the Claude Code → Groq regression of #764 ([#3258](https://github.com/diegosouzapw/OmniRoute/issues/3258))
|
||||
- **api/images:** `POST /v1/images/edits` to a custom OpenAI-compatible provider no longer forwards an empty `model`. The multipart body is now built as a `Buffer` with an explicit boundary instead of a global `FormData` — the patched undici `fetch` serialized a native `FormData` as the literal string `[object FormData]` (text/plain), dropping every field including `model` ([#3273](https://github.com/diegosouzapw/OmniRoute/issues/3273))
|
||||
- **api/webhooks:** webhook URLs may now target a private/internal address (e.g. `192.168.x`, a docker-internal host) when `OMNIROUTE_ALLOW_PRIVATE_PROVIDER_URLS=true` — the webhook guard reuses the same explicit opt-in as private provider URLs (default OFF; protocol and embedded-credential checks stay unconditional) ([#3269](https://github.com/diegosouzapw/OmniRoute/issues/3269))
|
||||
|
||||
---
|
||||
|
||||
|
||||
@@ -13,7 +13,7 @@ import { validateBody, isValidationFailure } from "@/shared/validation/helpers";
|
||||
import { requireManagementAuth } from "@/lib/api/requireManagementAuth";
|
||||
import { encryptMetadata } from "@/lib/webhookDispatcher";
|
||||
import { isEncryptionEnabled } from "@/lib/db/encryption";
|
||||
import { parseAndValidatePublicUrl } from "@/shared/network/outboundUrlGuard";
|
||||
import { parseAndValidateWebhookUrl } from "@/shared/network/outboundUrlGuard";
|
||||
|
||||
const WEBHOOK_KINDS = ["slack", "telegram", "discord", "custom"] as const;
|
||||
const WEBHOOK_EVENT_VALUES = [
|
||||
@@ -89,7 +89,7 @@ export async function PUT(request: Request, { params }: { params: Promise<{ id:
|
||||
|
||||
if (rest.url !== undefined && effectiveKind !== "telegram") {
|
||||
try {
|
||||
parseAndValidatePublicUrl(rest.url);
|
||||
parseAndValidateWebhookUrl(rest.url);
|
||||
} catch (err: any) {
|
||||
return NextResponse.json(
|
||||
{ error: err?.message || "Blocked private or invalid webhook URL" },
|
||||
|
||||
@@ -14,7 +14,7 @@ import { requireManagementAuth } from "@/lib/api/requireManagementAuth";
|
||||
import { insertDelivery } from "@/lib/db/webhookDeliveries";
|
||||
import { recordWebhookDelivery } from "@/lib/localDb";
|
||||
import {
|
||||
parseAndValidatePublicUrl,
|
||||
parseAndValidateWebhookUrl,
|
||||
OutboundUrlGuardError,
|
||||
} from "@/shared/network/outboundUrlGuard";
|
||||
import crypto from "crypto";
|
||||
@@ -34,7 +34,7 @@ async function testFetch(
|
||||
}> {
|
||||
const start = Date.now();
|
||||
try {
|
||||
parseAndValidatePublicUrl(url);
|
||||
parseAndValidateWebhookUrl(url);
|
||||
const controller = new AbortController();
|
||||
const timeoutId = setTimeout(() => controller.abort(), 10_000);
|
||||
const res = await fetch(url, {
|
||||
|
||||
@@ -12,7 +12,7 @@ import { validateBody, isValidationFailure } from "@/shared/validation/helpers";
|
||||
import { requireManagementAuth } from "@/lib/api/requireManagementAuth";
|
||||
import { encryptMetadata } from "@/lib/webhookDispatcher";
|
||||
import { isEncryptionEnabled } from "@/lib/db/encryption";
|
||||
import { parseAndValidatePublicUrl } from "@/shared/network/outboundUrlGuard";
|
||||
import { parseAndValidateWebhookUrl } from "@/shared/network/outboundUrlGuard";
|
||||
|
||||
const WEBHOOK_KINDS = ["slack", "telegram", "discord", "custom"] as const;
|
||||
|
||||
@@ -28,7 +28,7 @@ const createWebhookSchema = z
|
||||
.superRefine((data, ctx) => {
|
||||
if (data.kind === "telegram") return;
|
||||
try {
|
||||
parseAndValidatePublicUrl(data.url);
|
||||
parseAndValidateWebhookUrl(data.url);
|
||||
} catch (err: any) {
|
||||
ctx.addIssue({
|
||||
code: z.ZodIssueCode.custom,
|
||||
|
||||
@@ -7,7 +7,7 @@ import { z } from "zod";
|
||||
import { NextResponse } from "next/server";
|
||||
import { requireManagementAuth } from "@/lib/api/requireManagementAuth";
|
||||
import {
|
||||
parseAndValidatePublicUrl,
|
||||
parseAndValidateWebhookUrl,
|
||||
OutboundUrlGuardError,
|
||||
} from "@/shared/network/outboundUrlGuard";
|
||||
import { validateBody, isValidationFailure } from "@/shared/validation/helpers";
|
||||
@@ -35,7 +35,7 @@ export async function POST(request: Request) {
|
||||
const { url } = validation.data;
|
||||
|
||||
try {
|
||||
parseAndValidatePublicUrl(url);
|
||||
parseAndValidateWebhookUrl(url);
|
||||
return NextResponse.json({ valid: true });
|
||||
} catch (err) {
|
||||
if (err instanceof OutboundUrlGuardError) {
|
||||
|
||||
@@ -6,7 +6,7 @@
|
||||
|
||||
import crypto from "crypto";
|
||||
import { encrypt, decrypt } from "./db/encryption";
|
||||
import { parseAndValidatePublicUrl } from "@/shared/network/outboundUrlGuard";
|
||||
import { parseAndValidateWebhookUrl } from "@/shared/network/outboundUrlGuard";
|
||||
import type { WebhookEvent } from "./webhooks/eventDescriptions";
|
||||
|
||||
export type { WebhookEvent };
|
||||
@@ -42,7 +42,7 @@ async function deliverRaw(
|
||||
): Promise<{ success: boolean; status: number; latencyMs: number; error?: string }> {
|
||||
const start = Date.now();
|
||||
try {
|
||||
parseAndValidatePublicUrl(url);
|
||||
parseAndValidateWebhookUrl(url);
|
||||
const controller = new AbortController();
|
||||
const timeoutId = setTimeout(() => controller.abort(), 10_000);
|
||||
const res = await fetch(url, {
|
||||
@@ -70,7 +70,7 @@ export async function deliverWebhook(
|
||||
maxRetries = 3
|
||||
): Promise<{ success: boolean; status: number; error?: string }> {
|
||||
try {
|
||||
parseAndValidatePublicUrl(url);
|
||||
parseAndValidateWebhookUrl(url);
|
||||
} catch (error: any) {
|
||||
return { success: false, status: 0, error: error.message || "Blocked outbound URL" };
|
||||
}
|
||||
|
||||
@@ -125,6 +125,27 @@ export function parseAndValidatePublicUrl(input: string | URL) {
|
||||
return url;
|
||||
}
|
||||
|
||||
/**
|
||||
* Webhook variant of {@link parseAndValidatePublicUrl}. Webhooks legitimately point at
|
||||
* internal services (n8n, Home Assistant, a LAN box) in Docker/self-hosted deployments,
|
||||
* so the private-host block is gated behind the same explicit opt-in used for private
|
||||
* provider URLs (`OMNIROUTE_ALLOW_PRIVATE_PROVIDER_URLS`, default OFF). Protocol and
|
||||
* embedded-credential checks in {@link parseOutboundUrl} remain unconditional. (#3269)
|
||||
*/
|
||||
export function parseAndValidateWebhookUrl(input: string | URL) {
|
||||
const url = parseOutboundUrl(input);
|
||||
|
||||
if (!arePrivateProviderUrlsAllowed() && isPrivateHost(url.hostname)) {
|
||||
throw new OutboundUrlGuardError(PROVIDER_URL_BLOCKED_MESSAGE, {
|
||||
code: "OUTBOUND_URL_GUARD_BLOCKED",
|
||||
url: url.toString(),
|
||||
hostname: url.hostname || null,
|
||||
});
|
||||
}
|
||||
|
||||
return url;
|
||||
}
|
||||
|
||||
function isTrueValue(raw: unknown): boolean {
|
||||
if (typeof raw !== "string") return false;
|
||||
return TRUE_ENV_VALUES.has(raw.trim().toLowerCase());
|
||||
|
||||
82
tests/unit/webhook-private-optin-3269.test.ts
Normal file
82
tests/unit/webhook-private-optin-3269.test.ts
Normal file
@@ -0,0 +1,82 @@
|
||||
/**
|
||||
* #3269: webhooks could not target a private/internal address (10.x, 192.168.x, a
|
||||
* docker-internal host) — common for self-hosted automation (n8n, Home Assistant).
|
||||
* The webhook URL guard now allows private targets only when the operator opts in via
|
||||
* the existing `OMNIROUTE_ALLOW_PRIVATE_PROVIDER_URLS` flag (default OFF). Protocol and
|
||||
* embedded-credential checks stay unconditional.
|
||||
*/
|
||||
|
||||
import { describe, it, after } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
|
||||
process.env.DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omni-wh-3269-"));
|
||||
|
||||
const { parseAndValidateWebhookUrl, OutboundUrlGuardError } = await import(
|
||||
"../../src/shared/network/outboundUrlGuard.ts"
|
||||
);
|
||||
const { resetDbInstance } = await import("../../src/lib/db/core.ts");
|
||||
|
||||
const FLAG = "OMNIROUTE_ALLOW_PRIVATE_PROVIDER_URLS";
|
||||
|
||||
describe("parseAndValidateWebhookUrl — private target opt-in (#3269)", () => {
|
||||
it("blocks a private webhook URL when the opt-in is off (default)", () => {
|
||||
delete process.env[FLAG];
|
||||
assert.throws(
|
||||
() => parseAndValidateWebhookUrl("http://192.168.0.10/hook"),
|
||||
OutboundUrlGuardError
|
||||
);
|
||||
});
|
||||
|
||||
it("allows a private webhook URL when OMNIROUTE_ALLOW_PRIVATE_PROVIDER_URLS=true", () => {
|
||||
process.env[FLAG] = "true";
|
||||
try {
|
||||
const url = parseAndValidateWebhookUrl("http://192.168.0.10/hook");
|
||||
assert.equal(url.hostname, "192.168.0.10");
|
||||
} finally {
|
||||
delete process.env[FLAG];
|
||||
}
|
||||
});
|
||||
|
||||
it("still blocks embedded credentials even with the opt-in on", () => {
|
||||
process.env[FLAG] = "true";
|
||||
try {
|
||||
assert.throws(
|
||||
() => parseAndValidateWebhookUrl("http://user:pass@192.168.0.10/hook"),
|
||||
OutboundUrlGuardError
|
||||
);
|
||||
} finally {
|
||||
delete process.env[FLAG];
|
||||
}
|
||||
});
|
||||
|
||||
it("still blocks non-http(s) schemes even with the opt-in on", () => {
|
||||
process.env[FLAG] = "true";
|
||||
try {
|
||||
assert.throws(() => parseAndValidateWebhookUrl("file:///etc/passwd"), OutboundUrlGuardError);
|
||||
} finally {
|
||||
delete process.env[FLAG];
|
||||
}
|
||||
});
|
||||
|
||||
it("allows a normal public URL regardless of the flag", () => {
|
||||
delete process.env[FLAG];
|
||||
const url = parseAndValidateWebhookUrl("https://hooks.example.com/abc");
|
||||
assert.equal(url.hostname, "hooks.example.com");
|
||||
});
|
||||
});
|
||||
|
||||
after(() => {
|
||||
try {
|
||||
resetDbInstance();
|
||||
} catch {
|
||||
/* ignore */
|
||||
}
|
||||
try {
|
||||
fs.rmSync(process.env.DATA_DIR as string, { recursive: true, force: true });
|
||||
} catch {
|
||||
/* ignore */
|
||||
}
|
||||
});
|
||||
Reference in New Issue
Block a user