mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-03 22:02:08 +03:00
fix(api): LAN/Tailscale dashboard — host-aware CSP + GET-exempt version route + combo field errors (#5083) (#5177)
Integrated into release/v3.8.39. Host-aware CSP (ReDoS/injection-safe host validation) + GET-exempt /api/system/version (POST/spawn stays LOCAL_ONLY, exact-match safe-methods-only) + COMBO_002 firstField. 44/44 tests + route-guard membership gate green. CHANGELOG synced to keep all 4 v3.8.39 fixes.
This commit is contained in:
committed by
GitHub
parent
17e599b832
commit
ff392e694c
@@ -10,6 +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(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.
|
||||
|
||||
|
||||
@@ -117,10 +117,17 @@ export async function PUT(request, { params }) {
|
||||
const { id } = await params;
|
||||
const validation = validateBody(updateComboSchema, rawBody);
|
||||
if (isValidationFailure(validation)) {
|
||||
// Surface the first field-level issue so clients can highlight the
|
||||
// offending field without parsing the full issues array (#5083 Bug 3).
|
||||
const firstDetail = validation.error.details?.[0] ?? null;
|
||||
return comboErrorResponse(
|
||||
"COMBO_002",
|
||||
400,
|
||||
{ issues: validation.error },
|
||||
{
|
||||
issues: validation.error,
|
||||
firstField: firstDetail?.field ?? null,
|
||||
firstMessage: firstDetail?.message ?? null,
|
||||
},
|
||||
request
|
||||
);
|
||||
}
|
||||
|
||||
65
src/middleware.ts
Normal file
65
src/middleware.ts
Normal file
@@ -0,0 +1,65 @@
|
||||
/**
|
||||
* 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).*)"],
|
||||
};
|
||||
@@ -144,7 +144,7 @@ export const managementPolicy: RoutePolicy = {
|
||||
//
|
||||
// Anonymous (no Bearer / invalid key / wrong scope / no session) requests
|
||||
// still hit the same 403 LOCAL_ONLY they did before.
|
||||
if (isLocalOnlyPath(path) && !isLoopbackRequest(ctx) && !isPrivateLanRequest(ctx)) {
|
||||
if (isLocalOnlyPath(path, ctx.request?.method) && !isLoopbackRequest(ctx) && !isPrivateLanRequest(ctx)) {
|
||||
if (isLocalOnlyBypassableByManageScope(path)) {
|
||||
// Management auth is header-only — a URL-borne token must never satisfy a
|
||||
// manage-scope bypass of a LOCAL_ONLY route. See #3300 follow-up.
|
||||
|
||||
@@ -165,7 +165,46 @@ export function isPrivateLanHost(hostHeader: string | null): boolean {
|
||||
return PRIVATE_LAN_PATTERNS.some((re) => re.test(host));
|
||||
}
|
||||
|
||||
export function isLocalOnlyPath(path: string): boolean {
|
||||
/**
|
||||
* Paths that are LOCAL_ONLY for all write methods but may be accessed from
|
||||
* non-loopback clients when the request method is GET, HEAD, or OPTIONS.
|
||||
*
|
||||
* Rule: a path belongs here only when the read methods perform NO child-process
|
||||
* spawn and expose NO privileged mutation — only the write methods do.
|
||||
*
|
||||
* Current exemptions:
|
||||
* /api/system/version — GET reads package.json + npm registry; only POST
|
||||
* triggers the auto-update flow (spawns git checkout + npm install + pm2).
|
||||
* Hard Rules #15/#17 still apply to POST.
|
||||
*/
|
||||
export const LOCAL_ONLY_API_GET_EXEMPTIONS: ReadonlySet<string> = new Set([
|
||||
"/api/system/version",
|
||||
]);
|
||||
|
||||
/** Safe HTTP methods that can be exempted for read-only paths. */
|
||||
const SAFE_METHODS = new Set(["GET", "HEAD", "OPTIONS"]);
|
||||
|
||||
/**
|
||||
* Returns true when `path` is a local-only route that must be blocked from
|
||||
* non-loopback / non-LAN callers.
|
||||
*
|
||||
* @param path Normalized request path (e.g. "/api/mcp/sse").
|
||||
* @param method Optional HTTP method. When provided and the method is a safe
|
||||
* read-only method (GET/HEAD/OPTIONS) AND the path exactly
|
||||
* matches an entry in `LOCAL_ONLY_API_GET_EXEMPTIONS`, this
|
||||
* function returns false — i.e. the path is NOT local-only for
|
||||
* that specific safe method. With no method argument (e.g.
|
||||
* from security-scan scripts that test paths without a method),
|
||||
* the function returns true (safe default) to preserve the
|
||||
* conservative classification used by `check-route-guard-membership`.
|
||||
*/
|
||||
export function isLocalOnlyPath(path: string, method?: string): boolean {
|
||||
// Method-aware GET exemption: only exact-match paths in the exemption set
|
||||
// are eligible; prefix/wildcard matching is intentionally NOT used to avoid
|
||||
// accidentally opening sub-paths of a spawn-capable route.
|
||||
if (method && SAFE_METHODS.has(method.toUpperCase()) && LOCAL_ONLY_API_GET_EXEMPTIONS.has(path)) {
|
||||
return false;
|
||||
}
|
||||
return (
|
||||
LOCAL_ONLY_API_PREFIXES.some((p) => path === p || path.startsWith(p)) ||
|
||||
LOCAL_ONLY_API_PATTERNS.some((re) => re.test(path))
|
||||
|
||||
116
src/server/csp.ts
Normal file
116
src/server/csp.ts
Normal file
@@ -0,0 +1,116 @@
|
||||
/**
|
||||
* 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("; ");
|
||||
}
|
||||
109
tests/unit/api/combo-002-first-field.test.ts
Normal file
109
tests/unit/api/combo-002-first-field.test.ts
Normal file
@@ -0,0 +1,109 @@
|
||||
/**
|
||||
* TDD regression guard for #5083 — Bug 3:
|
||||
* COMBO_002 validation errors only surface the generic
|
||||
* "One or more combo fields are invalid" message; the field-level reason
|
||||
* is buried inside error.details.issues.details[*].
|
||||
*
|
||||
* Fix: extract the first issue from error.details and add
|
||||
* error.details.firstField / error.details.firstMessage to the response body.
|
||||
*/
|
||||
|
||||
import { describe, it } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { buildComboErrorBody } from "@/lib/api/comboErrorResponse";
|
||||
|
||||
/**
|
||||
* Simulate what the PUT /api/combos/[id] route passes as `details`
|
||||
* for a COMBO_002 response after the fix.
|
||||
* The fix extracts the first Zod issue from validation.error.details
|
||||
* and adds firstField / firstMessage to the payload.
|
||||
*/
|
||||
describe("COMBO_002 response — firstField / firstMessage surfacing (#5083 Bug 3)", () => {
|
||||
it("details payload exposes firstField when provided", () => {
|
||||
// Simulate the fixed route calling comboErrorResponse with firstField/firstMessage
|
||||
const details = {
|
||||
issues: { message: "Invalid request", details: [{ field: "name", message: "Required" }] },
|
||||
firstField: "name",
|
||||
firstMessage: "Required",
|
||||
};
|
||||
const body = buildComboErrorBody("COMBO_002", details);
|
||||
assert.equal(body.error.code, "COMBO_002");
|
||||
assert.equal(body.error.details.firstField, "name");
|
||||
assert.equal(body.error.details.firstMessage, "Required");
|
||||
});
|
||||
|
||||
it("details payload exposes firstField for nested path (e.g. models.0.id)", () => {
|
||||
const details = {
|
||||
issues: {
|
||||
message: "Invalid request",
|
||||
details: [{ field: "models.0.id", message: "String must contain at least 1 character(s)" }],
|
||||
},
|
||||
firstField: "models.0.id",
|
||||
firstMessage: "String must contain at least 1 character(s)",
|
||||
};
|
||||
const body = buildComboErrorBody("COMBO_002", details);
|
||||
assert.equal(body.error.details.firstField, "models.0.id");
|
||||
assert.equal(body.error.details.firstMessage, "String must contain at least 1 character(s)");
|
||||
});
|
||||
|
||||
it("details payload has firstField=null when no issues are present", () => {
|
||||
// Edge case: empty issues list
|
||||
const details = {
|
||||
issues: { message: "Invalid request", details: [] },
|
||||
firstField: null,
|
||||
firstMessage: null,
|
||||
};
|
||||
const body = buildComboErrorBody("COMBO_002", details);
|
||||
assert.equal(body.error.details.firstField, null);
|
||||
assert.equal(body.error.details.firstMessage, null);
|
||||
});
|
||||
|
||||
it("the generic message key is still present (backward compat)", () => {
|
||||
const details = {
|
||||
issues: { message: "Invalid request", details: [{ field: "strategy", message: "Invalid enum value" }] },
|
||||
firstField: "strategy",
|
||||
firstMessage: "Invalid enum value",
|
||||
};
|
||||
const body = buildComboErrorBody("COMBO_002", details);
|
||||
// The top-level error.message is from the error code catalog, not the issue message
|
||||
assert.equal(body.error.message, "One or more combo fields are invalid");
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* This helper simulates the logic that the fixed route.ts uses to extract
|
||||
* firstField/firstMessage from validateBody's error payload.
|
||||
* It exercises the extraction logic independently of the heavy route harness.
|
||||
*/
|
||||
describe("COMBO_002 firstField extraction logic", () => {
|
||||
function extractFirstField(validationError: {
|
||||
message: string;
|
||||
details: Array<{ field: string; message: string }>;
|
||||
}): { firstField: string | null; firstMessage: string | null } {
|
||||
const first = validationError.details?.[0] ?? null;
|
||||
return {
|
||||
firstField: first?.field ?? null,
|
||||
firstMessage: first?.message ?? null,
|
||||
};
|
||||
}
|
||||
|
||||
it("extracts the first field from a non-empty details array", () => {
|
||||
const error = {
|
||||
message: "Invalid request",
|
||||
details: [
|
||||
{ field: "name", message: "Required" },
|
||||
{ field: "strategy", message: "Invalid enum value" },
|
||||
],
|
||||
};
|
||||
const { firstField, firstMessage } = extractFirstField(error);
|
||||
assert.equal(firstField, "name");
|
||||
assert.equal(firstMessage, "Required");
|
||||
});
|
||||
|
||||
it("returns null/null for an empty details array", () => {
|
||||
const error = { message: "Invalid request", details: [] };
|
||||
const { firstField, firstMessage } = extractFirstField(error);
|
||||
assert.equal(firstField, null);
|
||||
assert.equal(firstMessage, null);
|
||||
});
|
||||
});
|
||||
97
tests/unit/authz/route-guard-version-get-exemption.test.ts
Normal file
97
tests/unit/authz/route-guard-version-get-exemption.test.ts
Normal file
@@ -0,0 +1,97 @@
|
||||
/**
|
||||
* TDD regression guard for #5083 — Bug 2:
|
||||
* GET /api/system/version is blocked from LAN/remote hosts because the entire
|
||||
* path is in LOCAL_ONLY_API_PREFIXES for all methods. Only POST spawns child
|
||||
* processes (git/npm/pm2); GET only reads package.json + npm registry.
|
||||
*
|
||||
* Fix: isLocalOnlyPath(path, method) returns false for safe HTTP methods
|
||||
* when the path exactly matches LOCAL_ONLY_API_GET_EXEMPTIONS.
|
||||
*
|
||||
* Security invariant: POST /api/system/version MUST remain local-only.
|
||||
* All OTHER local-only prefixes (/api/mcp/, /api/services/, etc.) must
|
||||
* remain local-only for GET too (exemption is exact-match only).
|
||||
*/
|
||||
|
||||
import { test, describe } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import {
|
||||
isLocalOnlyPath,
|
||||
LOCAL_ONLY_API_GET_EXEMPTIONS,
|
||||
} from "../../../src/server/authz/routeGuard.ts";
|
||||
|
||||
describe("isLocalOnlyPath — GET exemption for /api/system/version (#5083)", () => {
|
||||
// ── EXEMPTION APPLIES ──────────────────────────────────────────────────────
|
||||
|
||||
test("GET /api/system/version is NOT local-only (no child process spawn)", () => {
|
||||
assert.equal(isLocalOnlyPath("/api/system/version", "GET"), false);
|
||||
});
|
||||
|
||||
test("HEAD /api/system/version is NOT local-only (read-only method)", () => {
|
||||
assert.equal(isLocalOnlyPath("/api/system/version", "HEAD"), false);
|
||||
});
|
||||
|
||||
test("OPTIONS /api/system/version is NOT local-only (CORS preflight)", () => {
|
||||
assert.equal(isLocalOnlyPath("/api/system/version", "OPTIONS"), false);
|
||||
});
|
||||
|
||||
// ── SPAWN-CAPABLE METHODS REMAIN BLOCKED ──────────────────────────────────
|
||||
|
||||
test("POST /api/system/version STAYS local-only (spawns git/npm/pm2)", () => {
|
||||
assert.equal(isLocalOnlyPath("/api/system/version", "POST"), true);
|
||||
});
|
||||
|
||||
test("PUT /api/system/version stays local-only", () => {
|
||||
assert.equal(isLocalOnlyPath("/api/system/version", "PUT"), true);
|
||||
});
|
||||
|
||||
test("PATCH /api/system/version stays local-only", () => {
|
||||
assert.equal(isLocalOnlyPath("/api/system/version", "PATCH"), true);
|
||||
});
|
||||
|
||||
test("DELETE /api/system/version stays local-only", () => {
|
||||
assert.equal(isLocalOnlyPath("/api/system/version", "DELETE"), true);
|
||||
});
|
||||
|
||||
// ── SAFE DEFAULT: no method arg → still blocked ─────────────────────────
|
||||
|
||||
test("isLocalOnlyPath('/api/system/version') with NO method arg returns true (safe default)", () => {
|
||||
// Scripts like check-route-guard-membership call without a method; safe default
|
||||
// must be true so spawn-capable paths are never accidentally unblocked.
|
||||
assert.equal(isLocalOnlyPath("/api/system/version"), true);
|
||||
});
|
||||
|
||||
// ── EXEMPTION IS EXACT-MATCH ONLY ─────────────────────────────────────────
|
||||
|
||||
test("GET /api/system/version/extra is NOT exempted (prefix would be too broad)", () => {
|
||||
// The exemption applies only to the exact path — sub-paths are NOT exempted.
|
||||
assert.equal(isLocalOnlyPath("/api/system/version/extra", "GET"), true);
|
||||
});
|
||||
|
||||
// ── OTHER LOCAL-ONLY PREFIXES UNAFFECTED BY GET EXEMPTION ─────────────────
|
||||
|
||||
test("GET /api/mcp/ still local-only — exemption is NOT applied to /api/mcp/", () => {
|
||||
assert.equal(isLocalOnlyPath("/api/mcp/sse", "GET"), true);
|
||||
});
|
||||
|
||||
test("GET /api/services/9router/start still local-only", () => {
|
||||
assert.equal(isLocalOnlyPath("/api/services/9router/start", "GET"), true);
|
||||
});
|
||||
|
||||
test("GET /api/cli-tools/runtime/claude still local-only", () => {
|
||||
assert.equal(isLocalOnlyPath("/api/cli-tools/runtime/claude", "GET"), true);
|
||||
});
|
||||
|
||||
test("GET /api/db-backups/exportAll still local-only (spawns tar)", () => {
|
||||
assert.equal(isLocalOnlyPath("/api/db-backups/exportAll", "GET"), true);
|
||||
});
|
||||
|
||||
// ── EXEMPTION SET IS EXPORTED AND CONTAINS EXACTLY /api/system/version ───
|
||||
|
||||
test("LOCAL_ONLY_API_GET_EXEMPTIONS contains /api/system/version", () => {
|
||||
assert.ok(LOCAL_ONLY_API_GET_EXEMPTIONS.has("/api/system/version"));
|
||||
});
|
||||
|
||||
test("LOCAL_ONLY_API_GET_EXEMPTIONS has exactly 1 entry", () => {
|
||||
assert.equal(LOCAL_ONLY_API_GET_EXEMPTIONS.size, 1);
|
||||
});
|
||||
});
|
||||
163
tests/unit/csp-host-aware.test.ts
Normal file
163
tests/unit/csp-host-aware.test.ts
Normal file
@@ -0,0 +1,163 @@
|
||||
/**
|
||||
* 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);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user