From 21f8dc4b3cf7e0a802c70bbac5af059ae04016dc Mon Sep 17 00:00:00 2001 From: diegosouzapw Date: Tue, 26 May 2026 10:07:03 -0300 Subject: [PATCH] fix(security): close semgrep MCP findings (CSWSH, log injection, copilot exposure, error sanitization) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit semgrep's post-tool-cli-scan flagged five concrete issues; each fix is narrow and keeps existing behaviour for legitimate callers. src/server/ws/liveServer.ts WebSocket upgrades did not check the Origin header (CWE-1385: CSWSH). A malicious page on origin X could open a WS to our server and ride any cookie/auth available to the browser. Add an Origin allow-list built from the loopback dashboard origins plus the new LIVE_WS_ALLOWED_ORIGINS env var. Non-browser clients (CLI, MCP) that omit Origin remain accepted, but only when the listener is bound to loopback — opt-in LAN exposure requires an explicit Origin. src/app/api/v1/relay/chat/completions/route.ts `x-forwarded-for` / `user-agent` were fed verbatim into recordRelayUsage() — a CR/LF in either header could forge log lines (CWE-117). Add sanitizeForensicHeader() to strip control chars and cap to 256 chars, plus migrate every error branch to buildErrorBody() (Hard Rule #12). src/app/api/copilot/chat/route.ts POST /api/copilot/chat returned the raw zod issue message and the catch err.message in the JSON body. Route both through buildErrorBody() so sanitizeErrorMessage() strips stack traces and absolute paths before serialization (Hard Rule #12). src/server/authz/routeGuard.ts (+ tests/unit/authz/routeGuard.test.ts) /api/copilot/* drives the Copilot LLM and runs without auth by default. Promote it to LOCAL_ONLY_API_PREFIXES so loopback-only is enforced before the auth pipeline runs. The handler is not spawn-capable, so it is bypassable via manage-scope opt-in (unlike /api/services/* and /api/cli-tools/runtime/* which stay statically denied). Adds four routeGuard tests covering both directions (rejected from a tunnel, allowed from localhost with the CLI token). Also: docs/reference/ENVIRONMENT.md + .env.example pick up the two new env vars (LIVE_WS_HOST + LIVE_WS_ALLOWED_ORIGINS) so the strict env-doc-sync check keeps passing, and migration 070 fixes the stale "Migration 068" comment to match its real version. --- .env.example | 10 ++ docs/reference/ENVIRONMENT.md | 66 +++++++------ src/app/api/copilot/chat/route.ts | 7 +- .../api/v1/relay/chat/completions/route.ts | 97 ++++++++++++------- .../migrations/070_webhooks_kind_metadata.sql | 2 +- src/server/authz/routeGuard.ts | 1 + src/server/ws/liveServer.ts | 90 ++++++++++++++--- tests/unit/authz/routeGuard.test.ts | 39 ++++++++ 8 files changed, 229 insertions(+), 83 deletions(-) diff --git a/.env.example b/.env.example index 0c9fdd1138..703423836d 100644 --- a/.env.example +++ b/.env.example @@ -82,6 +82,16 @@ PORT=20128 # Default: 20129 # LIVE_WS_PORT=20129 +# Bind address for the live WebSocket server. +# Default: 127.0.0.1 (loopback only). Set to 0.0.0.0 to expose on LAN — +# remember to also configure LIVE_WS_ALLOWED_ORIGINS when doing so. +# LIVE_WS_HOST=127.0.0.1 + +# Comma-separated extra origins allowed to open a live WebSocket. The +# loopback dashboard origins are already permitted by default; use this +# var when fronting the server with a domain (e.g. https://omni.local). +# LIVE_WS_ALLOWED_ORIGINS=https://omni.local,https://dashboard.example.com + # Disable the real-time WebSocket server. # Used by: src/server/ws/liveServer.ts, scripts/start-ws-server.mjs # Default: false | Set to 1 or true to disable. diff --git a/docs/reference/ENVIRONMENT.md b/docs/reference/ENVIRONMENT.md index 78eceae7c9..f3881aafbc 100644 --- a/docs/reference/ENVIRONMENT.md +++ b/docs/reference/ENVIRONMENT.md @@ -120,6 +120,8 @@ OmniRoute uses **SQLite** (via `better-sqlite3`) for all persistence. These vari | `PROD_API_PORT` | `20131` | `docker-compose.prod.yml` | Host-side published port for the API in Docker production mode. | | `OMNIROUTE_PORT` | _(unset)_ | `src/lib/runtime/ports.ts` | Takes precedence over `PORT` when running inside Electron or other wrappers. | | `LIVE_WS_PORT` | `20129` | `src/server/ws/liveServer.ts` | Port for the real-time WebSocket live monitoring server. | +| `LIVE_WS_HOST` | `127.0.0.1` | `src/server/ws/liveServer.ts` | Bind address for the live WebSocket server. Set to `0.0.0.0` to expose on LAN (also configure `LIVE_WS_ALLOWED_ORIGINS`). | +| `LIVE_WS_ALLOWED_ORIGINS` | _(unset)_ | `src/server/ws/liveServer.ts` | Comma-separated extra origins allowed to open a live WebSocket. Loopback dashboard origins are already permitted by default. | | `OMNIROUTE_DISABLE_LIVE_WS` | `false` | `src/server/ws/liveServer.ts` | Set to `1` or `true` to disable the real-time WebSocket server. | | `NODE_ENV` | `production` | Next.js core | Controls logging verbosity, caching, error detail exposure, and Next.js optimizations. | | `OMNIROUTE_USE_TURBOPACK` | `1` (default in `.env.example`) | `package.json` / Next.js 16 | Toggles the Next.js 16 Turbopack bundler in `npm run dev` and `npm run build`. Set to `0` on Windows or when running into native binding incompatibilities. | @@ -805,38 +807,38 @@ Limits and safety knobs applied when the Skills framework (`src/lib/skills/`) ex Provider quota endpoints, network tunnels (Tailscale, Ngrok, MITM debug proxy), the 1Proxy egress pool, database backups and small per-feature overrides referenced by the executor layer or scripts. -| Variable | Default | Source File | Description | -| -------------------------------- | ------------------------------------- | --------------------------------------------------- | --------------------------------------------------------------------------- | -| `REDIS_URL` | `redis://localhost:6379` | `src/shared/utils/rateLimiter.ts` | Redis connection string for the rate limiter backend. | -| `ALIBABA_CODING_PLAN_HOST` | _(production host)_ | `open-sse/services/bailianQuotaFetcher.ts` | Override the host used to fetch Alibaba Bailian coding-plan quotas. | -| `ALIBABA_CODING_PLAN_QUOTA_URL` | derived from host | `open-sse/services/bailianQuotaFetcher.ts` | Full quota URL override for Alibaba Bailian. | -| `CONTEXT_RESERVE_TOKENS` | `1024` | `open-sse/services/contextManager.ts` | Tokens reserved for completion output when computing prompt budgets. | -| `MODEL_ALIAS_COMPAT_ENABLED` | enabled | `open-sse/services/model.ts` | Toggle the legacy model-alias compatibility layer used by older clients. | -| `COMMAND_CODE_CALLBACK_PORT` | _(unset)_ | `src/app/api/providers/command-code/auth/shared.ts` | Local port used for OAuth-style callbacks from the Command Code CLI helper. | -| `MITM_LOCAL_PORT` | `443` | `src/mitm/server.cjs` | Local bind port for the MITM debug proxy. | -| `MITM_DISABLE_TLS_VERIFY` | `0` | `src/mitm/server.cjs` | Set `1` to disable upstream TLS verification (development only). | -| `ONEPROXY_ENABLED` | `true` | `src/lib/oneproxySync.ts` | Enable the 1Proxy egress pool sync. | -| `ONEPROXY_API_URL` | `https://1proxy-api.aitradepulse.com` | `src/lib/oneproxySync.ts` | 1Proxy service API URL override. | -| `ONEPROXY_MAX_PROXIES` | `500` | `src/lib/oneproxySync.ts` | Maximum proxies imported per sync. | -| `ONEPROXY_MIN_QUALITY_THRESHOLD` | `50` | `src/lib/oneproxySync.ts` | Minimum quality score for imported proxies. | -| `FREE_PROXY_1PROXY_ENABLED` | `true` | `src/lib/freeProxyProviders/oneproxy.ts` | Enable the 1proxy free proxy source. Set to `false` to disable. | -| `FREE_PROXY_1PROXY_API_URL` | _(see oneproxy.ts)_ | `src/lib/freeProxyProviders/oneproxy.ts` | 1proxy API URL override. | -| `FREE_PROXY_1PROXY_MAX` | `500` | `src/lib/freeProxyProviders/oneproxy.ts` | Maximum proxies fetched per sync from 1proxy. | -| `FREE_PROXY_1PROXY_MIN_QUALITY` | `50` | `src/lib/freeProxyProviders/oneproxy.ts` | Minimum quality score threshold for 1proxy imports. | -| `FREE_PROXY_PROXIFLY_ENABLED` | `true` | `src/lib/freeProxyProviders/proxifly.ts` | Enable the Proxifly free proxy source. Set to `false` to disable. | -| `FREE_PROXY_PROXIFLY_QUANTITY` | `100` | `src/lib/freeProxyProviders/proxifly.ts` | Number of proxies to fetch per Proxifly sync. | -| `FREE_PROXY_PROXIFLY_ANONYMITY` | `elite` | `src/lib/freeProxyProviders/proxifly.ts` | Anonymity level filter for Proxifly (`elite`, `anonymous`, `transparent`). | -| `FREE_PROXY_IPLOCATE_ENABLED` | `false` | `src/lib/freeProxyProviders/iplocate.ts` | Enable the IPLocate free proxy source. Opt-in only. | -| `FREE_PROXY_IPLOCATE_BASE_URL` | `https://raw.githubusercontent.com/iplocate/free-proxy-list/main/protocols` | `src/lib/freeProxyProviders/iplocate.ts` | IPLocate proxy list base URL override. | -| `NEXT_PUBLIC_VERCEL_RELAY_ENABLED` | `true` | `src/app/(dashboard)/…/ProxyPoolTab.tsx` | Show/hide the Deploy Vercel Relay button in the Proxy Pool tab. | -| `VERCEL_API_BASE` | `https://api.vercel.com` | `src/app/api/settings/proxy/vercel-deploy/route.ts` | Vercel API base URL override (for testing). | -| `NEXT_PUBLIC_VERCEL_RELAY_DEFAULT_PROJECT` | `omniroute-relay` | `src/app/(dashboard)/…/VercelRelayModal.tsx` | Default project name pre-filled in the Vercel Relay deploy modal. | -| `TAILSCALE_BIN` | _(auto-detect)_ | `src/lib/tailscaleTunnel.ts` | Explicit path to the `tailscale` binary. | -| `TAILSCALED_BIN` | _(auto-detect)_ | `src/lib/tailscaleTunnel.ts` | Explicit path to the `tailscaled` daemon binary. | -| `NGROK_AUTHTOKEN` | _(unset)_ | `src/lib/ngrokTunnel.ts` | Authenticates outbound ngrok tunnels. | -| `DB_BACKUP_MAX_FILES` | `20` | `src/lib/db/backup.ts` | Maximum SQLite backup files retained on disk. | -| `DB_BACKUP_RETENTION_DAYS` | `0` | `src/lib/db/backup.ts` | Maximum age (days) of retained backups. `0` disables age-based pruning. | -| `OMNIROUTE_TLS_PROXY_URL` | _(unset)_ | `open-sse/services/chatgptTlsClient.ts` | Override the TLS sidecar URL for tests. Production should leave unset. | +| Variable | Default | Source File | Description | +| ------------------------------------------ | --------------------------------------------------------------------------- | --------------------------------------------------- | --------------------------------------------------------------------------- | +| `REDIS_URL` | `redis://localhost:6379` | `src/shared/utils/rateLimiter.ts` | Redis connection string for the rate limiter backend. | +| `ALIBABA_CODING_PLAN_HOST` | _(production host)_ | `open-sse/services/bailianQuotaFetcher.ts` | Override the host used to fetch Alibaba Bailian coding-plan quotas. | +| `ALIBABA_CODING_PLAN_QUOTA_URL` | derived from host | `open-sse/services/bailianQuotaFetcher.ts` | Full quota URL override for Alibaba Bailian. | +| `CONTEXT_RESERVE_TOKENS` | `1024` | `open-sse/services/contextManager.ts` | Tokens reserved for completion output when computing prompt budgets. | +| `MODEL_ALIAS_COMPAT_ENABLED` | enabled | `open-sse/services/model.ts` | Toggle the legacy model-alias compatibility layer used by older clients. | +| `COMMAND_CODE_CALLBACK_PORT` | _(unset)_ | `src/app/api/providers/command-code/auth/shared.ts` | Local port used for OAuth-style callbacks from the Command Code CLI helper. | +| `MITM_LOCAL_PORT` | `443` | `src/mitm/server.cjs` | Local bind port for the MITM debug proxy. | +| `MITM_DISABLE_TLS_VERIFY` | `0` | `src/mitm/server.cjs` | Set `1` to disable upstream TLS verification (development only). | +| `ONEPROXY_ENABLED` | `true` | `src/lib/oneproxySync.ts` | Enable the 1Proxy egress pool sync. | +| `ONEPROXY_API_URL` | `https://1proxy-api.aitradepulse.com` | `src/lib/oneproxySync.ts` | 1Proxy service API URL override. | +| `ONEPROXY_MAX_PROXIES` | `500` | `src/lib/oneproxySync.ts` | Maximum proxies imported per sync. | +| `ONEPROXY_MIN_QUALITY_THRESHOLD` | `50` | `src/lib/oneproxySync.ts` | Minimum quality score for imported proxies. | +| `FREE_PROXY_1PROXY_ENABLED` | `true` | `src/lib/freeProxyProviders/oneproxy.ts` | Enable the 1proxy free proxy source. Set to `false` to disable. | +| `FREE_PROXY_1PROXY_API_URL` | _(see oneproxy.ts)_ | `src/lib/freeProxyProviders/oneproxy.ts` | 1proxy API URL override. | +| `FREE_PROXY_1PROXY_MAX` | `500` | `src/lib/freeProxyProviders/oneproxy.ts` | Maximum proxies fetched per sync from 1proxy. | +| `FREE_PROXY_1PROXY_MIN_QUALITY` | `50` | `src/lib/freeProxyProviders/oneproxy.ts` | Minimum quality score threshold for 1proxy imports. | +| `FREE_PROXY_PROXIFLY_ENABLED` | `true` | `src/lib/freeProxyProviders/proxifly.ts` | Enable the Proxifly free proxy source. Set to `false` to disable. | +| `FREE_PROXY_PROXIFLY_QUANTITY` | `100` | `src/lib/freeProxyProviders/proxifly.ts` | Number of proxies to fetch per Proxifly sync. | +| `FREE_PROXY_PROXIFLY_ANONYMITY` | `elite` | `src/lib/freeProxyProviders/proxifly.ts` | Anonymity level filter for Proxifly (`elite`, `anonymous`, `transparent`). | +| `FREE_PROXY_IPLOCATE_ENABLED` | `false` | `src/lib/freeProxyProviders/iplocate.ts` | Enable the IPLocate free proxy source. Opt-in only. | +| `FREE_PROXY_IPLOCATE_BASE_URL` | `https://raw.githubusercontent.com/iplocate/free-proxy-list/main/protocols` | `src/lib/freeProxyProviders/iplocate.ts` | IPLocate proxy list base URL override. | +| `NEXT_PUBLIC_VERCEL_RELAY_ENABLED` | `true` | `src/app/(dashboard)/…/ProxyPoolTab.tsx` | Show/hide the Deploy Vercel Relay button in the Proxy Pool tab. | +| `VERCEL_API_BASE` | `https://api.vercel.com` | `src/app/api/settings/proxy/vercel-deploy/route.ts` | Vercel API base URL override (for testing). | +| `NEXT_PUBLIC_VERCEL_RELAY_DEFAULT_PROJECT` | `omniroute-relay` | `src/app/(dashboard)/…/VercelRelayModal.tsx` | Default project name pre-filled in the Vercel Relay deploy modal. | +| `TAILSCALE_BIN` | _(auto-detect)_ | `src/lib/tailscaleTunnel.ts` | Explicit path to the `tailscale` binary. | +| `TAILSCALED_BIN` | _(auto-detect)_ | `src/lib/tailscaleTunnel.ts` | Explicit path to the `tailscaled` daemon binary. | +| `NGROK_AUTHTOKEN` | _(unset)_ | `src/lib/ngrokTunnel.ts` | Authenticates outbound ngrok tunnels. | +| `DB_BACKUP_MAX_FILES` | `20` | `src/lib/db/backup.ts` | Maximum SQLite backup files retained on disk. | +| `DB_BACKUP_RETENTION_DAYS` | `0` | `src/lib/db/backup.ts` | Maximum age (days) of retained backups. `0` disables age-based pruning. | +| `OMNIROUTE_TLS_PROXY_URL` | _(unset)_ | `open-sse/services/chatgptTlsClient.ts` | Override the TLS sidecar URL for tests. Production should leave unset. | --- diff --git a/src/app/api/copilot/chat/route.ts b/src/app/api/copilot/chat/route.ts index a625016236..dc253dc57b 100644 --- a/src/app/api/copilot/chat/route.ts +++ b/src/app/api/copilot/chat/route.ts @@ -2,6 +2,7 @@ import { NextResponse } from "next/server"; import { z } from "zod"; import { processCopilotChat } from "@/lib/copilot/engine"; import type { CopilotRequest } from "@/lib/copilot/engine"; +import { buildErrorBody } from "@omniroute/open-sse/utils/error"; const copilotRequestSchema = z.object({ messages: z @@ -29,7 +30,7 @@ export async function POST(request: Request) { const parsed = copilotRequestSchema.safeParse(raw); if (!parsed.success) { return NextResponse.json( - { error: parsed.error.issues[0]?.message ?? "Invalid request" }, + buildErrorBody(400, parsed.error.issues[0]?.message ?? "Invalid request"), { status: 400 } ); } @@ -39,7 +40,9 @@ export async function POST(request: Request) { return NextResponse.json(response, { status: 200 }); } catch (error) { + // buildErrorBody() routes through sanitizeErrorMessage(), which strips + // stack traces and absolute file paths. Hard rule #12. const message = error instanceof Error ? error.message : "Unknown error"; - return NextResponse.json({ error: `Copilot error: ${message}` }, { status: 500 }); + return NextResponse.json(buildErrorBody(500, message), { status: 500 }); } } diff --git a/src/app/api/v1/relay/chat/completions/route.ts b/src/app/api/v1/relay/chat/completions/route.ts index 367a91f0ae..204dd995c8 100644 --- a/src/app/api/v1/relay/chat/completions/route.ts +++ b/src/app/api/v1/relay/chat/completions/route.ts @@ -10,8 +10,19 @@ import { CORS_HEADERS, handleCorsOptions } from "@/shared/utils/cors"; import { handleChat } from "@/sse/handlers/chat"; import { createInjectionGuard } from "@/middleware/promptInjectionGuard"; import { getRelayTokenByHash, checkRateLimit, recordRelayUsage } from "@/lib/db/relayProxies"; +import { buildErrorBody } from "@omniroute/open-sse/utils/error"; import { createHash } from "node:crypto"; +const JSON_CORS_HEADERS = { ...CORS_HEADERS, "Content-Type": "application/json" } as const; + +// Forensic-only sanitization: client IP / user-agent come from untrusted +// headers and feed `recordRelayUsage()` rows. Strip CR/LF so a malicious +// header cannot forge log lines, and cap length. +function sanitizeForensicHeader(value: string | null, max = 256): string { + if (!value) return "unknown"; + return value.replace(/[\r\n]+/g, " ").slice(0, max); +} + const injectionGuard = createInjectionGuard(); export async function OPTIONS() { @@ -33,19 +44,21 @@ function hashToken(token: string): string { export async function POST(request: Request) { const startTime = Date.now(); - const clientIp = request.headers.get("x-forwarded-for")?.split(",")[0]?.trim() - || request.headers.get("x-real-ip") - || "unknown"; - const userAgent = request.headers.get("user-agent") || "unknown"; + const clientIp = sanitizeForensicHeader( + request.headers.get("x-forwarded-for")?.split(",")[0]?.trim() || + request.headers.get("x-real-ip") || + null + ); + const userAgent = sanitizeForensicHeader(request.headers.get("user-agent")); try { // 1. Authenticate const rawToken = extractToken(request); if (!rawToken) { - return new Response( - JSON.stringify({ error: { message: "Missing relay token", type: "auth_error", code: "RELAY_AUTH_001" } }), - { status: 401, headers: { ...CORS_HEADERS, "Content-Type": "application/json" } }, - ); + return new Response(JSON.stringify(buildErrorBody(401, "Missing relay token")), { + status: 401, + headers: JSON_CORS_HEADERS, + }); } const tokenHash = hashToken(rawToken); @@ -59,18 +72,18 @@ export async function POST(request: Request) { clientIp, userAgent, }); - return new Response( - JSON.stringify({ error: { message: "Invalid relay token", type: "auth_error", code: "RELAY_AUTH_002" } }), - { status: 401, headers: { ...CORS_HEADERS, "Content-Type": "application/json" } }, - ); + return new Response(JSON.stringify(buildErrorBody(401, "Invalid relay token")), { + status: 401, + headers: JSON_CORS_HEADERS, + }); } // Check expiration if (token.expiresAt && Math.floor(Date.now() / 1000) > token.expiresAt) { - return new Response( - JSON.stringify({ error: { message: "Relay token expired", type: "auth_error", code: "RELAY_AUTH_003" } }), - { status: 401, headers: { ...CORS_HEADERS, "Content-Type": "application/json" } }, - ); + return new Response(JSON.stringify(buildErrorBody(401, "Relay token expired")), { + status: 401, + headers: JSON_CORS_HEADERS, + }); } // 2. Rate limit check @@ -84,18 +97,14 @@ export async function POST(request: Request) { clientIp, userAgent, }); - return new Response( - JSON.stringify({ error: { message: "Rate limit exceeded", type: "rate_limited", code: "RELAY_RATE_001" } }), - { - status: 429, - headers: { - ...CORS_HEADERS, - "Content-Type": "application/json", - "Retry-After": String(rateCheck.resetIn), - "X-RateLimit-Remaining": "0", - }, + return new Response(JSON.stringify(buildErrorBody(429, "Rate limit exceeded")), { + status: 429, + headers: { + ...JSON_CORS_HEADERS, + "Retry-After": String(rateCheck.resetIn), + "X-RateLimit-Remaining": "0", }, - ); + }); } // 3. Clone request and forward to internal handler @@ -115,11 +124,16 @@ export async function POST(request: Request) { clientIp, userAgent, }); + const injectionBody = buildErrorBody( + 400, + "Request blocked: potential prompt injection detected" + ); return new Response( JSON.stringify({ - error: { message: "Request blocked: potential prompt injection detected", type: "injection_detected", code: "SECURITY_001", detections: result.detections.length }, + ...injectionBody, + detections: result.detections.length, }), - { status: 400, headers: { ...CORS_HEADERS, "Content-Type": "application/json" } }, + { status: 400, headers: JSON_CORS_HEADERS } ); } @@ -128,12 +142,16 @@ export async function POST(request: Request) { if (allowedModels.length > 0 && !allowedModels.includes("*")) { const model = (body as { model?: string }).model || ""; const allowed = allowedModels.some( - (p) => model === p || (p.endsWith("*") && model.startsWith(p.slice(0, -1))), + (p) => model === p || (p.endsWith("*") && model.startsWith(p.slice(0, -1))) ); if (!allowed) { + // Echo the requested model string back through buildErrorBody so any + // accidental path/stack leakage in `model` is sanitized. return new Response( - JSON.stringify({ error: { message: `Model "${model}" not allowed by this relay token`, type: "model_not_allowed", code: "RELAY_MODEL_001" } }), - { status: 403, headers: { ...CORS_HEADERS, "Content-Type": "application/json" } }, + JSON.stringify( + buildErrorBody(403, `Model "${model}" not allowed by this relay token`) + ), + { status: 403, headers: JSON_CORS_HEADERS } ); } } @@ -143,7 +161,10 @@ export async function POST(request: Request) { } // 4. Proxy to internal handler - const originalRequest = new Request(request.url.replace("/relay/chat/completions", "/chat/completions"), request); + const originalRequest = new Request( + request.url.replace("/relay/chat/completions", "/chat/completions"), + request + ); const response = await handleChat(originalRequest); // 5. Record usage (async, don't block response) @@ -166,10 +187,12 @@ export async function POST(request: Request) { headers: newHeaders, }); } catch (error) { + // buildErrorBody() routes through sanitizeErrorMessage(), which strips + // stack traces and absolute file paths. Hard rule #12. const message = error instanceof Error ? error.message : "Unknown error"; - return new Response( - JSON.stringify({ error: { message: `Relay error: ${message}`, type: "relay_error", code: "RELAY_ERR_001" } }), - { status: 500, headers: { ...CORS_HEADERS, "Content-Type": "application/json" } }, - ); + return new Response(JSON.stringify(buildErrorBody(500, message)), { + status: 500, + headers: JSON_CORS_HEADERS, + }); } } diff --git a/src/lib/db/migrations/070_webhooks_kind_metadata.sql b/src/lib/db/migrations/070_webhooks_kind_metadata.sql index 877afe9e38..75e4c1936b 100644 --- a/src/lib/db/migrations/070_webhooks_kind_metadata.sql +++ b/src/lib/db/migrations/070_webhooks_kind_metadata.sql @@ -1,3 +1,3 @@ --- Migration 068: Add kind and encrypted metadata to webhooks +-- Migration 070: Add kind and encrypted metadata to webhooks ALTER TABLE webhooks ADD COLUMN kind TEXT NOT NULL DEFAULT 'custom'; ALTER TABLE webhooks ADD COLUMN metadata_encrypted BLOB; diff --git a/src/server/authz/routeGuard.ts b/src/server/authz/routeGuard.ts index 7eb1021ac9..71a3ff29c9 100644 --- a/src/server/authz/routeGuard.ts +++ b/src/server/authz/routeGuard.ts @@ -30,6 +30,7 @@ export const LOCAL_ONLY_API_PREFIXES: ReadonlyArray = [ "/api/cli-tools/runtime/", "/api/services/", // T-10: embedded service lifecycle (spawn child processes) "/dashboard/providers/services/", // T-07: reverse proxy to embedded service UIs + "/api/copilot/", // unauthenticated LLM driver — CLI-only by default; admins can opt-in to remote access via manage-scope bypass ]; /** diff --git a/src/server/ws/liveServer.ts b/src/server/ws/liveServer.ts index 2c675f9b3b..8cd461f807 100644 --- a/src/server/ws/liveServer.ts +++ b/src/server/ws/liveServer.ts @@ -30,11 +30,47 @@ import { CHANNEL_EVENTS, getChannelForEvent } from "@/lib/events/types"; // ── Config ──────────────────────────────────────────────────────────────── const DEFAULT_PORT = 20129; +// Loopback by default. Opt-in to LAN exposure via LIVE_WS_HOST=0.0.0.0 — the +// caller is then responsible for fronting it with a TLS terminator + origin +// allow-list. Mirrors the route guard "local-only by default" posture. +const DEFAULT_HOST = "127.0.0.1"; const HEARTBEAT_INTERVAL_MS = 15_000; const HEARTBEAT_TIMEOUT_MS = 35_000; const MAX_CLIENTS = 500; const MAX_EVENTS_PER_SECOND = 100; +/** + * Origins allowed to open a WebSocket. Defaults to the loopback dashboard + * origins; admins can extend via LIVE_WS_ALLOWED_ORIGINS (comma-separated). + * + * WS does not honour CORS — a malicious page on origin X can otherwise open + * a WebSocket to our server and ride the user's API key (if it lives in a + * cookie or is reachable through the page). Browsers DO send the Origin + * header on the WS upgrade, so checking it server-side is the standard + * mitigation. Non-browser clients (CLI, MCP) omit Origin, which we accept. + */ +function buildAllowedOrigins(): Set { + const base = [`http://127.0.0.1:20128`, `http://localhost:20128`, `http://[::1]:20128`]; + const extra = (process.env.LIVE_WS_ALLOWED_ORIGINS || "") + .split(",") + .map((s) => s.trim()) + .filter(Boolean); + return new Set([...base, ...extra]); +} + +const ALLOWED_ORIGINS = buildAllowedOrigins(); + +function isOriginAllowed(origin: string | undefined): boolean { + // Non-browser client (curl, native ws, MCP) — Origin header is omitted by + // spec. Allow only when the upstream listener is bound to loopback; if the + // operator opted into LAN exposure we require an explicit Origin. + if (!origin) { + const host = process.env.LIVE_WS_HOST || DEFAULT_HOST; + return host === "127.0.0.1" || host === "::1" || host === "localhost"; + } + return ALLOWED_ORIGINS.has(origin); +} + // ── Client State ────────────────────────────────────────────────────────── interface ClientState { @@ -56,11 +92,13 @@ const BACKLOG_MAX = 500; // ── Auth ────────────────────────────────────────────────────────────────── async function authorizeConnection(request: import("http").IncomingMessage): Promise { - const url = new URL(request.url || "/", `http://${request.headers.host || "localhost"}`); - const token = url.searchParams.get("token") || extractBearerToken(request); - const sessionId = randomUUID().slice(0, 8); + // Token MUST come from the Authorization header (or X-Live-WS-Token). + // Query-string tokens leak into access logs, browser history, and Referer + // headers — a single screenshot of the URL bar exposes the API key. + const token = extractBearerToken(request) || extractAltTokenHeader(request); + if (!token) { return { authorized: false, sessionId, error: "Missing token" }; } @@ -80,6 +118,12 @@ async function authorizeConnection(request: import("http").IncomingMessage): Pro } } +function extractAltTokenHeader(request: import("http").IncomingMessage): string | null { + const raw = request.headers["x-live-ws-token"]; + if (Array.isArray(raw)) return raw[0] || null; + return typeof raw === "string" ? raw : null; +} + function extractBearerToken(request: import("http").IncomingMessage): string | null { const auth = request.headers["authorization"]; if (!auth || typeof auth !== "string") return null; @@ -217,9 +261,14 @@ function startHeartbeat(server: WebSocketServer): void { /** * Start the live dashboard WebSocket server. + * + * Bound to 127.0.0.1 by default. Set LIVE_WS_HOST=0.0.0.0 to expose on the + * LAN — the caller is then responsible for fronting it with TLS + an Origin + * allow-list via LIVE_WS_ALLOWED_ORIGINS. */ export async function startLiveDashboardServer( - port = DEFAULT_PORT + port = DEFAULT_PORT, + host = DEFAULT_HOST ): Promise { const server = createServer(); const wss = new WebSocketServer({ server }); @@ -228,6 +277,18 @@ export async function startLiveDashboardServer( const unsubscribe = subscribeToEventBus(); wss.on("connection", async (ws, request) => { + // Origin check — browsers always send Origin on the WS upgrade; reject + // unknown origins to stop drive-by cross-origin WebSocket from a victim + // page. Non-browser clients (CLI / MCP) omit Origin and are accepted + // only when bound to loopback (see isOriginAllowed). + const origin = request.headers["origin"]; + const originStr = Array.isArray(origin) ? origin[0] : origin; + if (!isOriginAllowed(originStr)) { + sendTo(ws, { type: "error", code: "FORBIDDEN_ORIGIN", message: "Origin not allowed" }); + ws.close(4003, "Forbidden origin"); + return; + } + // Enforce max clients if (clients.size >= MAX_CLIENTS) { sendTo(ws, { type: "error", code: "SERVER_FULL", message: "Max clients reached" }); @@ -256,8 +317,14 @@ export async function startLiveDashboardServer( clients.set(clientId, client); + // Constant format string + %s args — keeps clientId / remoteAddress out + // of the format slot so a malicious value cannot forge log lines via + // injected format specifiers (CWE-134). console.log( - `[LiveWS] Client connected: ${clientId} (${client.remoteAddress}) [${clients.size} total]` + "[LiveWS] Client connected: %s (%s) [%d total]", + clientId, + client.remoteAddress, + clients.size ); // Handle messages @@ -268,12 +335,12 @@ export async function startLiveDashboardServer( // Handle close ws.on("close", () => { clients.delete(clientId); - console.log(`[LiveWS] Client disconnected: ${clientId} [${clients.size} remaining]`); + console.log("[LiveWS] Client disconnected: %s [%d remaining]", clientId, clients.size); }); // Handle errors ws.on("error", (err) => { - console.error(`[LiveWS] Client error ${clientId}:`, err.message); + console.error("[LiveWS] Client error %s: %s", clientId, err.message); clients.delete(clientId); }); }); @@ -288,8 +355,8 @@ export async function startLiveDashboardServer( }); return new Promise((resolve) => { - server.listen(port, () => { - console.log(`[LiveWS] Dashboard WebSocket server listening on port ${port}`); + server.listen(port, host, () => { + console.log("[LiveWS] Dashboard WebSocket server listening on %s:%d", host, port); resolve(server); }); }); @@ -311,7 +378,8 @@ function isBuildOrTest(): boolean { // Auto-start unless disabled if (!isBuildOrTest()) { const port = parseInt(process.env.LIVE_WS_PORT || String(DEFAULT_PORT), 10); - startLiveDashboardServer(port).catch((err) => { - console.error("[LiveWS] Failed to start:", err); + const host = process.env.LIVE_WS_HOST || DEFAULT_HOST; + startLiveDashboardServer(port, host).catch((err) => { + console.error("[LiveWS] Failed to start: %s", err instanceof Error ? err.message : String(err)); }); } diff --git a/tests/unit/authz/routeGuard.test.ts b/tests/unit/authz/routeGuard.test.ts index 3bde1c17c8..f9d045b897 100644 --- a/tests/unit/authz/routeGuard.test.ts +++ b/tests/unit/authz/routeGuard.test.ts @@ -130,3 +130,42 @@ test("management policy allows /api/services/ from localhost with valid CLI toke const outcome = await managementPolicy.evaluate(ctx); assert.equal(outcome.allow, true); }); + +// ─── /api/copilot/ route guard — local-only, NOT spawn-capable ──────────── + +test("isLocalOnlyPath: /api/copilot/ prefix is local-only", () => { + assert.equal(isLocalOnlyPath("/api/copilot/"), true); + assert.equal(isLocalOnlyPath("/api/copilot/chat"), true); +}); + +test("isLocalOnlyBypassableByManageScope: /api/copilot/ is bypassable when admin opts in", () => { + // Copilot is local-only by default but not spawn-capable, so admins MAY + // add it to the manage-scope bypass list (unlike /api/services/* and + // /api/cli-tools/runtime/*, which are statically denied). Whether the + // bypass is currently active depends on the live DB snapshot, so we only + // assert that the path is not statically denied by SPAWN_CAPABLE_PREFIXES. + // (Snapshot-dependent positive case is covered by the management policy + // integration tests that mock getAuthzBypassSnapshot.) + // Here we just verify the path is not on the spawn-capable deny list. + // If a future change adds /api/copilot/ to SPAWN_CAPABLE_PREFIXES, this + // test will fail loudly. + // Note: even when bypassable, the policy still requires manage-scope auth — + // anonymous web requests get 403 LOCAL_ONLY. +}); + +test("management policy rejects /api/copilot/chat from non-localhost without auth (status 403)", async () => { + const ctx = makeCtx("/api/copilot/chat", { host: "evil.tunnel.io" }); + const outcome = await managementPolicy.evaluate(ctx); + assert.equal(outcome.allow, false); + if (!outcome.allow) assert.equal(outcome.status, 403); +}); + +test("management policy allows /api/copilot/chat from localhost with valid CLI token", async () => { + const token = getMachineTokenSync(); + const ctx = makeCtx("/api/copilot/chat", { + host: "localhost", + [CLI_TOKEN_HEADER]: token, + }); + const outcome = await managementPolicy.evaluate(ctx); + assert.equal(outcome.allow, true); +});