pjandro/OmniRoute
1
0
Fork 0
mirror of https://github.com/diegosouzapw/OmniRoute.git synced 2026-07-31 04:12:10 +03:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #5182 from diegosouzapw/fix/5083-csp-no-middleware-followup

Browse Source 
fix(api): replace #5083 global middleware CSP with declarative ws: scheme
This commit is contained in:
Diego Rodrigues de Sa e Souza
2026-06-27 13:19:07 -03:00
committed by GitHub
parent ff392e694c 4ce87e8c30
commit b1cf8f7955
6 changed files with 77 additions and 346 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
CHANGELOG.md
View File

@@ -10,7 +10,7 @@ _In development — bullets added per PR; finalized at release._
### 🔧 Bug Fixes
- **fix(api): LAN/Tailscale dashboard access — host-aware CSP, GET-exempt version route, surface combo field errors** — three failures when opening the dashboard from a non-loopback host: (1) CSP `connect-src` was a static loopback-only string, blocking `ws://<lan-host>:*` WebSocket connections from LAN/Tailscale clients; the CSP is now built per-request from a validated `Host` header (`src/server/csp.ts` + `src/middleware.ts`) with a strict hostname/IPv4 regex so injection-shaped values are never interpolated; (2) `GET /api/system/version` was blocked by `LOCAL_ONLY_API_PREFIXES` for all methods despite only `POST` spawning child processes (git/npm/pm2) — a new `LOCAL_ONLY_API_GET_EXEMPTIONS` set exempts safe read methods for this path while keeping `POST`/`PUT`/`PATCH`/`DELETE` strictly loopback-only; (3) `COMBO_002` validation errors only surfaced the generic message — `firstField`/`firstMessage` are now extracted from the first Zod issue and included in the response body. ([#5083](https://github.com/diegosouzapw/OmniRoute/issues/5083) — thanks @KooshaPari for the diagnosis and original PR #5084)
- **fix(api): LAN/Tailscale dashboard access — `ws:` CSP scheme, GET-exempt version route, surface combo field errors** — three failures when opening the dashboard from a non-loopback host: (1) CSP `connect-src` allowed the `ws:` scheme only for loopback origins, blocking the dashboard's `ws://<lan-host>:*` Live WebSocket from LAN/Tailscale clients; the bare `ws:` scheme is now permitted (symmetric with the bare `wss:` already allowed), kept declarative in `next.config.mjs` with no global middleware (the project has none by design); (2) `GET /api/system/version` was blocked by `LOCAL_ONLY_API_PREFIXES` for all methods despite only `POST` spawning child processes (git/npm/pm2) — a new `LOCAL_ONLY_API_GET_EXEMPTIONS` set exempts safe read methods for this path while keeping `POST`/`PUT`/`PATCH`/`DELETE` strictly loopback-only; (3) `COMBO_002` validation errors only surfaced the generic message — `firstField`/`firstMessage` are now extracted from the first Zod issue and included in the response body. ([#5083](https://github.com/diegosouzapw/OmniRoute/issues/5083) — thanks @KooshaPari for the diagnosis and original PR #5084)
- **fix(sse): defer `</think>` close so it never leaks before `tool_calls` in Claude→OpenAI streaming** — when a Claude thinking block was followed by a tool_use block, the translator unconditionally emitted a `content: "</think>"` chunk at `content_block_stop`, injecting a spurious assistant text chunk immediately before the `tool_calls` delta and corrupting OpenAI-compatible clients (e.g. Kimi Coding). The close marker is now deferred: it is flushed at the first `text_delta` that follows the thinking block (preserving the #4633 / decolua/9router#454 behavior for Claude Code / Cursor) or at stream finish when no tool_calls were collected. Tool-use streams never get a `text_delta` after the thinking block, so `</think>` is never emitted into content before `tool_calls`. ([#5123](https://github.com/diegosouzapw/OmniRoute/issues/5123))
- **fix(sse): normalize array user-message content in the Command Code executor to prevent upstream 400** — when a client sends a user turn whose `content` is an array of content parts (e.g. `[{type:"text",text:"…"}, …]`), the raw array was forwarded verbatim to the Command Code upstream, which requires `messages[N].content` for the `user` role to be a plain string — resulting in `expected string, received array` / HTTP 400 on DeepSeek V4-Pro and other Command Code models. The user branch of `convertMessages` now calls `normalizeContentText()` (already used by system, assistant, and tool branches) so multi-part user content is joined to a string before dispatch. Partially addresses ([#5166](https://github.com/diegosouzapw/OmniRoute/issues/5166)); the 0-output-token symptom on reasoning-only models is tracked separately.

6
next.config.mjs
View File

@@ -21,7 +21,11 @@ const contentSecurityPolicy = [
"font-src 'self' https://fonts.gstatic.com data:",
"img-src 'self' data: blob: https:",
"media-src 'self' data: blob:",
"connect-src 'self' http://localhost:* http://127.0.0.1:* ws://localhost:* ws://127.0.0.1:* https: wss:",
// `ws:` is permitted scheme-wide (mirroring the bare `wss:` already allowed) so the
// dashboard can open `ws://<lan-or-tailscale-host>:*` to its own Live WS server when
// OmniRoute is reached from a non-loopback host. Same-origin HTTP fetches stay covered
// by `'self'`; the loopback origins remain listed explicitly for clarity. (#5083)
"connect-src 'self' http://localhost:* http://127.0.0.1:* ws://localhost:* ws://127.0.0.1:* https: ws: wss:",
"worker-src 'self' blob:",
"manifest-src 'self'",
].join("; ");

65
src/middleware.ts
View File

@@ -1,65 +0,0 @@
/**
* Next.js Edge Middleware — per-request Content-Security-Policy (#5083).
*
* The static CSP in next.config.mjs only covers loopback origins in
* connect-src. When OmniRoute is reached from a LAN, Tailscale, or public
* hostname the dashboard cannot establish WebSocket connections because
* ws://<hostname>:* is absent from the static policy.
*
* This middleware reads the trusted Host header, validates it with a strict
* regex, and — for valid non-loopback hosts — appends
* ws://<host>:* http://<host>:*
* to connect-src before the response reaches the browser.
*
* The CSP header set here overrides the static next.config.mjs header
* because middleware runs before the static route headers are applied.
* The static CSP is kept in next.config.mjs as a build-time fallback for
* environments where middleware is disabled.
*
* Security:
* - Host values are validated with a bounded hostname/IPv4 regex before
* interpolation. Invalid / injection-shaped values are ignored.
* - /dashboard/providers/services/*/embed/* keeps "frame-ancestors 'self'"
* (overrides the baseline "frame-ancestors 'none'") so the embedded
* service UI can be iframed by the OmniRoute dashboard.
* - All other hard-coded security directives (object-src, form-action …)
* are preserved verbatim from the baseline.
*/
import { NextResponse } from "next/server";
import type { NextRequest } from "next/server";
import { buildCspForHost } from "@/server/csp";
/** Path prefix for embedded service reverse-proxy pages (Hard Rule #17). */
const EMBED_PREFIX = "/dashboard/providers/services/";
export function middleware(request: NextRequest): NextResponse {
const response = NextResponse.next();
const { pathname } = request.nextUrl;
const host = request.headers.get("host");
// Embedded service UI pages need `frame-ancestors 'self'` so that the
// OmniRoute dashboard can render them inside an <iframe>. The route is
// already LOCAL_ONLY (routeGuard.ts) so non-loopback callers cannot reach
// it — the relaxed frame-ancestors is safe in that context.
if (pathname.startsWith(EMBED_PREFIX) && pathname.includes("/embed/")) {
response.headers.set("Content-Security-Policy", "frame-ancestors 'self'");
return response;
}
// All other paths: build a host-aware CSP.
response.headers.set("Content-Security-Policy", buildCspForHost(host));
return response;
}
export const config = {
/**
* Run on every page / API route.
* Excludes:
* _next/static — static asset chunks (no HTML/headers needed)
* _next/image — image optimisation endpoint
* favicon.ico — no CSP needed on icon requests
*/
matcher: ["/((?!_next/static|_next/image|favicon\\.ico).*)"],
};

116
src/server/csp.ts
View File

@@ -1,116 +0,0 @@
/**
* Per-request Content-Security-Policy builder (#5083).
*
* The static CSP in next.config.mjs only covers loopback origins.
* When OmniRoute is reached from a LAN, Tailscale, or public hostname the
* dashboard opens WebSocket connections to ws://<window.location.hostname>:*
* which the browser blocks because that host is absent from connect-src.
*
* Fix: build the CSP per-request by reading the validated Host header and
* appending ws://<host>:* / http://<host>:* to connect-src.
*
* Security constraints:
* - The Host header value is VALIDATED with a strict hostname/IPv4 regex
* before being interpolated into any CSP directive. An invalid or
* injection-shaped Host (e.g. "evil.com ; script-src *") is silently
* ignored — the baseline CSP is returned unchanged.
* - Loopback hosts are excluded (already covered by the baseline).
* - Bounded quantifiers are used throughout to prevent ReDoS
* (CLAUDE.md §PII §1).
*/
/** Loopback hostnames that need no additional connect-src entry. */
const LOOPBACK_HOSTS = new Set(["localhost", "127.0.0.1", "::1"]);
/**
* Strict allow-regex for a hostname that is safe to inject into a CSP
* directive value. Accepts:
* • IPv4 — four octets of 1-3 decimal digits
* • Label — RFC-1123 hostname labels (alphanumeric + hyphen, 1-63 chars
* each), up to 10 labels joined by dots
* Explicitly rejects anything containing space, ";", "'", or other CSP
* meta-characters. Uses bounded repetition to prevent ReDoS.
*/
const VALID_HOST_RE =
/^(?:(?:\d{1,3}\.){3}\d{1,3}|(?:[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?\.){0,10}[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?)$/i;
/**
* Extract and validate the plain hostname from a raw `Host` header value
* (which may include a port, e.g. "192.168.1.5:20128").
*
* Returns the lower-cased hostname if it is a valid, non-loopback host that
* is safe to interpolate into a CSP directive; returns `null` otherwise.
*/
export function sanitizeHostForCsp(rawHost: string | null): string | null {
if (!rawHost) return null;
let host = rawHost.trim();
// Reject IPv6 literals — loopback [::1] is already in the baseline; other
// IPv6 LAN addresses are uncommon and harder to validate safely here.
if (host.startsWith("[")) return null;
// Strip trailing :port for IPv4 / plain hostname (a single colon).
// A bare IPv6 address has multiple colons; we already rejected "[" above.
const colonCount = (host.match(/:/g) ?? []).length;
if (colonCount === 1) {
host = host.split(":")[0];
} else if (colonCount > 1) {
// Unbracketed multi-colon string — likely a bare IPv6 address or
// injection attempt; reject.
return null;
}
host = host.toLowerCase();
if (LOOPBACK_HOSTS.has(host)) return null; // already in baseline
if (!VALID_HOST_RE.test(host)) return null; // invalid / injection attempt
return host;
}
/**
* The baseline connect-src value that always applies.
* Covers loopback HTTP + WS, plus bare https:/wss: for external API calls.
*/
const BASELINE_CONNECT_SRC =
"connect-src 'self' http://localhost:* http://127.0.0.1:* ws://localhost:* ws://127.0.0.1:* https: wss:";
/**
* Ordered CSP directive parts that form the baseline policy.
* Kept here so unit tests can assert individual directives without
* parsing the full concatenated string.
*/
export const CSP_BASELINE_PARTS: ReadonlyArray<string> = [
"default-src 'self'",
"base-uri 'self'",
"object-src 'none'",
"frame-ancestors 'none'",
"form-action 'self'",
"script-src 'self' 'unsafe-inline' 'unsafe-eval' blob:",
"style-src 'self' 'unsafe-inline' https://fonts.googleapis.com",
"font-src 'self' https://fonts.gstatic.com data:",
"img-src 'self' data: blob: https:",
"media-src 'self' data: blob:",
BASELINE_CONNECT_SRC,
"worker-src 'self' blob:",
"manifest-src 'self'",
];
/**
* Build a per-request Content-Security-Policy that extends the loopback
* baseline with `ws://<host>:*` and `http://<host>:*` when the Host header
* carries a validated non-loopback hostname or IPv4 address.
*
* @param rawHost Value of the `Host` request header (may include port).
* `null` is safe — returns the loopback baseline unchanged.
*/
export function buildCspForHost(rawHost: string | null): string {
const host = sanitizeHostForCsp(rawHost);
if (!host) {
return CSP_BASELINE_PARTS.join("; ");
}
// Inject the validated host into connect-src only — never into other directives.
const parts = CSP_BASELINE_PARTS.map((p) =>
p.startsWith("connect-src ") ? `${p} ws://${host}:* http://${host}:*` : p
);
return parts.join("; ");
}

163
tests/unit/csp-host-aware.test.ts
View File

@@ -1,163 +0,0 @@
/**
* TDD regression guard for #5083 — Bug 1:
* CSP connect-src is a static string that only covers loopback origins.
* When the dashboard is accessed from a LAN or Tailscale IP the browser
* blocks WebSocket connections because ws://<lan-host>:* is absent.
*
* Fix: build the CSP per-request; validate the Host header and append
* ws://<host>:* / http://<host>:* to connect-src when the host is a
* valid non-loopback hostname or IPv4 address.
*
* Security invariant: a Host header containing injection characters
* (semicolon, space, quotes, …) must NEVER be interpolated into the CSP.
*/
import { describe, it } from "node:test";
import assert from "node:assert/strict";
import {
sanitizeHostForCsp,
buildCspForHost,
CSP_BASELINE_PARTS,
} from "../../src/server/csp.ts";
describe("sanitizeHostForCsp — host validation", () => {
it("returns null for null input", () => {
assert.equal(sanitizeHostForCsp(null), null);
});
it("returns null for empty string", () => {
assert.equal(sanitizeHostForCsp(""), null);
});
it("returns null for localhost (already in baseline)", () => {
assert.equal(sanitizeHostForCsp("localhost"), null);
assert.equal(sanitizeHostForCsp("localhost:20128"), null);
});
it("returns null for 127.0.0.1 (loopback)", () => {
assert.equal(sanitizeHostForCsp("127.0.0.1"), null);
assert.equal(sanitizeHostForCsp("127.0.0.1:3000"), null);
});
it("returns null for [::1] IPv6 loopback", () => {
assert.equal(sanitizeHostForCsp("[::1]"), null);
});
it("returns the host for a private LAN IPv4 (192.168.x.x)", () => {
assert.equal(sanitizeHostForCsp("192.168.1.100"), "192.168.1.100");
assert.equal(sanitizeHostForCsp("192.168.1.100:20128"), "192.168.1.100");
});
it("returns the host for a Tailscale CGNAT IP (100.64.x.x)", () => {
assert.equal(sanitizeHostForCsp("100.64.0.1"), "100.64.0.1");
assert.equal(sanitizeHostForCsp("100.64.0.1:20128"), "100.64.0.1");
});
it("returns lower-cased hostname for a valid domain", () => {
assert.equal(sanitizeHostForCsp("my-server.local"), "my-server.local");
assert.equal(sanitizeHostForCsp("MY-SERVER.LOCAL:20128"), "my-server.local");
});
it("strips the port before validation", () => {
assert.equal(sanitizeHostForCsp("192.168.0.15:20128"), "192.168.0.15");
});
// ── SECURITY: injection / malicious inputs must be rejected ───────────────
it("rejects a Host with semicolons (CSP delimiter injection attempt)", () => {
assert.equal(sanitizeHostForCsp("evil.com ; script-src *"), null);
});
it("rejects a Host with spaces", () => {
assert.equal(sanitizeHostForCsp("evil host"), null);
});
it("rejects a Host with single quotes", () => {
assert.equal(sanitizeHostForCsp("evil.com' script-src *"), null);
});
it("rejects a Host with wildcard characters", () => {
assert.equal(sanitizeHostForCsp("*.evil.com"), null);
});
it("rejects a bare IPv6 address (unbracketed, multiple colons)", () => {
// e.g. "fe80::1" — multiple colons with no brackets
assert.equal(sanitizeHostForCsp("fe80::1"), null);
});
it("rejects a bracketed non-loopback IPv6 (not yet supported)", () => {
assert.equal(sanitizeHostForCsp("[fe80::1]"), null);
});
});
describe("buildCspForHost — per-request CSP", () => {
it("returns baseline CSP when host is null", () => {
const csp = buildCspForHost(null);
assert.ok(csp.includes("connect-src 'self' http://localhost:*"));
assert.ok(csp.includes("wss:"));
assert.ok(!csp.includes("ws://192"));
});
it("returns baseline CSP for loopback host", () => {
const csp = buildCspForHost("localhost:20128");
assert.ok(csp.includes("connect-src 'self' http://localhost:*"));
// Loopback should not be ADDED a second time
const connectSrc = csp.split(";").find((p) => p.trim().startsWith("connect-src"));
assert.ok(connectSrc);
// Count occurrences of "localhost" — should be exactly 2 (http + ws)
const localhostCount = (connectSrc.match(/localhost/g) ?? []).length;
assert.equal(localhostCount, 2);
});
it("appends ws://<host>:* and http://<host>:* for a valid LAN IPv4", () => {
const csp = buildCspForHost("100.64.0.1");
assert.ok(csp.includes("ws://100.64.0.1:*"), "should contain ws:// for LAN host");
assert.ok(csp.includes("http://100.64.0.1:*"), "should contain http:// for LAN host");
// baseline loopback origins must still be present
assert.ok(csp.includes("ws://localhost:*"), "loopback ws still present");
assert.ok(csp.includes("http://localhost:*"), "loopback http still present");
});
it("appends host entries for a valid domain (e.g. my-server.local)", () => {
const csp = buildCspForHost("my-server.local:20128");
assert.ok(csp.includes("ws://my-server.local:*"));
assert.ok(csp.includes("http://my-server.local:*"));
});
it("does NOT inject a malicious Host header into CSP", () => {
// The attacker-controlled Host header value contains CSP metacharacters.
const maliciousHost = "evil.com ; script-src *";
const csp = buildCspForHost(maliciousHost);
// The CSP must NOT contain the raw injection string
assert.ok(!csp.includes("evil.com"), "raw evil.com must not appear in CSP");
assert.ok(!csp.includes("; script-src *"), "injected directive must not appear");
// It should be identical to the baseline (malicious host rejected)
assert.equal(csp, buildCspForHost(null));
});
it("host additions appear ONLY in connect-src, not other directives", () => {
const csp = buildCspForHost("192.168.1.100");
const directives = csp.split(";").map((d) => d.trim());
for (const d of directives) {
if (d.startsWith("connect-src")) continue; // connect-src is expected to have the host
assert.ok(
!d.includes("192.168.1.100"),
`host must not appear in directive: ${d}`
);
}
});
it("baseline CSP parts are all present in output", () => {
const csp = buildCspForHost(null);
// Check a representative sample of baseline directives
assert.ok(csp.includes("default-src 'self'"));
assert.ok(csp.includes("frame-ancestors 'none'"));
assert.ok(csp.includes("object-src 'none'"));
assert.ok(csp.includes("script-src 'self' 'unsafe-inline'"));
});
it("CSP_BASELINE_PARTS array has a connect-src entry", () => {
const hasConnectSrc = CSP_BASELINE_PARTS.some((p) => p.startsWith("connect-src "));
assert.ok(hasConnectSrc);
});
});

71
tests/unit/csp-lan-ws-5083.test.ts Normal file
View File

@@ -0,0 +1,71 @@
// Regression guard for #5083 (Bug 1): the dashboard, when reached from a LAN /
// Tailscale / non-loopback host, opens `ws://<that-host>:<port>` to its own Live WS
// server. The static Content-Security-Policy in next.config.mjs previously allowed
// `ws:` only for loopback origins (`ws://localhost:*`, `ws://127.0.0.1:*`) plus a bare
// `wss:` — so a plain-`ws:` connection to a non-loopback host was blocked by the
// browser. The fix permits the bare `ws:` scheme (symmetric with the bare `wss:` that
// was already allowed), without introducing any global Next.js middleware (the project
// intentionally has none — interception is route-specific; see CLAUDE.md / AGENTS.md).
//
// This test pins the connect-src directive in next.config.mjs so the LAN/Tailscale WS
// allowance cannot silently regress, while confirming the other security directives are
// left intact.
import test from "node:test";
import assert from "node:assert/strict";
import { readFileSync } from "node:fs";
import { fileURLToPath } from "node:url";
import { dirname, resolve } from "node:path";
const here = dirname(fileURLToPath(import.meta.url));
const nextConfig = readFileSync(resolve(here, "../../next.config.mjs"), "utf8");
// Extract the connect-src directive string literal from the CSP array.
function connectSrcDirective(): string {
const match = nextConfig.match(/"(connect-src[^"]*)"/);
assert.ok(match, "next.config.mjs must define a connect-src directive in the CSP array");
return match![1];
}
test("#5083 connect-src permits the bare ws: scheme for non-loopback dashboards", () => {
const connectSrc = connectSrcDirective();
// Bare `ws:` must be present as its own token (not just `ws://localhost`).
assert.match(
connectSrc,
/(^|\s)ws:(\s|$)/,
`connect-src must allow the bare ws: scheme; got: ${connectSrc}`
);
});
test("#5083 connect-src keeps the bare wss: scheme it mirrors", () => {
const connectSrc = connectSrcDirective();
assert.match(connectSrc, /(^|\s)wss:(\s|$)/, "connect-src must still allow bare wss:");
});
test("#5083 connect-src still scopes 'self' and explicit loopback origins", () => {
const connectSrc = connectSrcDirective();
assert.ok(connectSrc.includes("'self'"), "connect-src must still include 'self'");
assert.ok(
connectSrc.includes("ws://localhost:*") && connectSrc.includes("ws://127.0.0.1:*"),
"explicit loopback ws origins must remain listed"
);
});
test("#5083 fix does NOT introduce a global Next.js middleware", () => {
// The project has no global middleware by design; the CSP fix must stay declarative.
let exists = true;
try {
readFileSync(resolve(here, "../../src/middleware.ts"), "utf8");
} catch {
exists = false;
}
assert.equal(exists, false, "src/middleware.ts must not exist (no global middleware — see CLAUDE.md)");
});
test("#5083 baseline security directives remain intact in the CSP", () => {
// frame-ancestors 'none' is the global default; object-src 'none' and base-uri 'self'
// must not have been weakened by the connect-src change.
assert.ok(nextConfig.includes("frame-ancestors 'none'"), "frame-ancestors 'none' must remain");
assert.ok(nextConfig.includes("object-src 'none'"), "object-src 'none' must remain");
assert.ok(nextConfig.includes("base-uri 'self'"), "base-uri 'self' must remain");
});