diff --git a/changelog.d/fixes/11872-tunnel-mitm-error-sanitization.md b/changelog.d/fixes/11872-tunnel-mitm-error-sanitization.md new file mode 100644 index 0000000000..f402b9c8a3 --- /dev/null +++ b/changelog.d/fixes/11872-tunnel-mitm-error-sanitization.md @@ -0,0 +1 @@ +- **fix(api):** Stop the tunnel and MITM routes from returning raw child-process error text, which disclosed host paths, binary install locations and Tailscale `tskey-*` credentials in a body some of these routes serve to non-loopback callers, and return a real 400 instead of a framework 500 when a tunnel request body fails validation. diff --git a/src/app/api/settings/mitm/route.ts b/src/app/api/settings/mitm/route.ts index bbd5622455..72c8c7d1e5 100644 --- a/src/app/api/settings/mitm/route.ts +++ b/src/app/api/settings/mitm/route.ts @@ -5,6 +5,7 @@ import path from "path"; import { z } from "zod"; import { NextResponse } from "next/server"; import { requireManagementAuth } from "@/lib/api/requireManagementAuth"; +import { toPublicSafeTunnelError } from "@/lib/api/publicSafeTunnelError"; import { resolveApiKey } from "@/shared/services/apiKeyResolver"; import { resolveMitmDataDir } from "@/mitm/dataDir"; import { KIRO_MITM_PROFILE } from "@/mitm/targets/kiro"; @@ -180,8 +181,10 @@ export async function GET(request: Request) { return NextResponse.json(await buildMitmResponse()); } catch (error) { - const message = error instanceof Error ? error.message : "Failed to load MITM settings"; - return NextResponse.json({ error: message }, { status: 500 }); + return NextResponse.json( + toPublicSafeTunnelError(error, "Failed to load the MITM settings.", "settings/mitm GET"), + { status: 500 } + ); } } @@ -236,8 +239,10 @@ export async function PUT(request: Request) { return NextResponse.json(await buildMitmResponse()); } catch (error) { - const message = error instanceof Error ? error.message : "Failed to update MITM settings"; - return NextResponse.json({ error: message }, { status: 500 }); + return NextResponse.json( + toPublicSafeTunnelError(error, "Failed to update the MITM settings.", "settings/mitm PUT"), + { status: 500 } + ); } } @@ -274,8 +279,13 @@ export async function POST(request: Request) { return NextResponse.json(await buildMitmResponse()); } catch (error) { - const message = - error instanceof Error ? error.message : "Failed to regenerate MITM certificate"; - return NextResponse.json({ error: message }, { status: 500 }); + return NextResponse.json( + toPublicSafeTunnelError( + error, + "Failed to regenerate the MITM certificate.", + "settings/mitm POST" + ), + { status: 500 } + ); } } diff --git a/src/app/api/tunnels/cloudflared/route.ts b/src/app/api/tunnels/cloudflared/route.ts index 14f78fbdf5..b0aa08abbc 100644 --- a/src/app/api/tunnels/cloudflared/route.ts +++ b/src/app/api/tunnels/cloudflared/route.ts @@ -1,12 +1,17 @@ import { NextRequest, NextResponse } from "next/server"; import { z } from "zod"; import { isAuthenticated } from "@/shared/utils/apiAuth"; -import { isValidationFailure, validateBody } from "@/shared/validation/helpers"; +import { + formatValidationMessage, + isValidationFailure, + validateBody, +} from "@/shared/validation/helpers"; import { getCloudflaredTunnelStatus, startCloudflaredTunnel, stopCloudflaredTunnel, } from "@/lib/cloudflaredTunnel"; +import { toPublicSafeTunnelError } from "@/lib/api/publicSafeTunnelError"; export const dynamic = "force-dynamic"; @@ -28,9 +33,11 @@ export async function GET(request: NextRequest) { return NextResponse.json(status); } catch (error) { return NextResponse.json( - { - error: error instanceof Error ? error.message : "Failed to load cloudflared tunnel status", - }, + toPublicSafeTunnelError( + error, + "Failed to load the cloudflared tunnel status.", + "tunnels/cloudflared GET" + ), { status: 500 } ); } @@ -50,7 +57,10 @@ export async function POST(request: NextRequest) { const validation = validateBody(actionSchema, rawBody); if (isValidationFailure(validation)) { - return validation.response; + // validateBody() returns { success, error } — it has no `response` field, so + // the previous `return validation.response` returned undefined and Next + // answered with a framework 500 instead of this 400. + return NextResponse.json({ error: formatValidationMessage(validation.error) }, { status: 400 }); } const parsed = validation.data; @@ -66,9 +76,11 @@ export async function POST(request: NextRequest) { }); } catch (error) { return NextResponse.json( - { - error: error instanceof Error ? error.message : "Failed to update cloudflared tunnel", - }, + toPublicSafeTunnelError( + error, + "Failed to update the cloudflared tunnel.", + "tunnels/cloudflared POST" + ), { status: 500 } ); } diff --git a/src/app/api/tunnels/ngrok/route.ts b/src/app/api/tunnels/ngrok/route.ts index 5fad841674..1ad8824d35 100644 --- a/src/app/api/tunnels/ngrok/route.ts +++ b/src/app/api/tunnels/ngrok/route.ts @@ -1,8 +1,13 @@ import { NextRequest, NextResponse } from "next/server"; import { z } from "zod"; import { isAuthenticated } from "@/shared/utils/apiAuth"; -import { isValidationFailure, validateBody } from "@/shared/validation/helpers"; +import { + formatValidationMessage, + isValidationFailure, + validateBody, +} from "@/shared/validation/helpers"; import { getNgrokTunnelStatus, startNgrokTunnel, stopNgrokTunnel } from "@/lib/ngrokTunnel"; +import { toPublicSafeTunnelError } from "@/lib/api/publicSafeTunnelError"; export const dynamic = "force-dynamic"; @@ -25,9 +30,11 @@ export async function GET(request: NextRequest) { return NextResponse.json(status); } catch (error) { return NextResponse.json( - { - error: error instanceof Error ? error.message : "Failed to load ngrok tunnel status", - }, + toPublicSafeTunnelError( + error, + "Failed to load the ngrok tunnel status.", + "tunnels/ngrok GET" + ), { status: 500 } ); } @@ -47,7 +54,10 @@ export async function POST(request: NextRequest) { const validation = validateBody(actionSchema, rawBody); if (isValidationFailure(validation)) { - return validation.response; + // validateBody() returns { success, error } — it has no `response` field, so + // the previous `return validation.response` returned undefined and Next + // answered with a framework 500 instead of this 400. + return NextResponse.json({ error: formatValidationMessage(validation.error) }, { status: 400 }); } const parsed = validation.data; @@ -65,9 +75,7 @@ export async function POST(request: NextRequest) { }); } catch (error) { return NextResponse.json( - { - error: error instanceof Error ? error.message : "Failed to update ngrok tunnel", - }, + toPublicSafeTunnelError(error, "Failed to update the ngrok tunnel.", "tunnels/ngrok POST"), { status: 500 } ); } diff --git a/src/app/api/tunnels/tailscale/check/route.ts b/src/app/api/tunnels/tailscale/check/route.ts index 864c396be4..acdf4def34 100644 --- a/src/app/api/tunnels/tailscale/check/route.ts +++ b/src/app/api/tunnels/tailscale/check/route.ts @@ -1,5 +1,6 @@ import { NextResponse } from "next/server"; import { getTailscaleCheckStatus } from "@/lib/tailscaleTunnel"; +import { toPublicSafeTunnelError } from "@/lib/api/publicSafeTunnelError"; import { requireTailscaleAuth } from "../routeUtils"; export const dynamic = "force-dynamic"; @@ -13,9 +14,11 @@ export async function GET(request: Request) { return NextResponse.json(status); } catch (error) { return NextResponse.json( - { - error: error instanceof Error ? error.message : "Failed to check Tailscale state", - }, + toPublicSafeTunnelError( + error, + "Failed to check the Tailscale state.", + "tunnels/tailscale/check GET" + ), { status: 500 } ); } diff --git a/src/app/api/tunnels/tailscale/disable/route.ts b/src/app/api/tunnels/tailscale/disable/route.ts index 858e337037..a5f4cbd303 100644 --- a/src/app/api/tunnels/tailscale/disable/route.ts +++ b/src/app/api/tunnels/tailscale/disable/route.ts @@ -1,5 +1,6 @@ import { NextResponse } from "next/server"; import { disableTailscaleTunnel } from "@/lib/tailscaleTunnel"; +import { toPublicSafeTunnelError } from "@/lib/api/publicSafeTunnelError"; import { parseOptionalJsonBody, requireTailscaleAuth, tailscaleSudoSchema } from "../routeUtils"; export const dynamic = "force-dynamic"; @@ -16,9 +17,11 @@ export async function POST(request: Request) { return NextResponse.json(result); } catch (error) { return NextResponse.json( - { - error: error instanceof Error ? error.message : "Failed to disable Tailscale Funnel", - }, + toPublicSafeTunnelError( + error, + "Failed to disable the Tailscale Funnel.", + "tunnels/tailscale/disable POST" + ), { status: 500 } ); } diff --git a/src/app/api/tunnels/tailscale/enable/route.ts b/src/app/api/tunnels/tailscale/enable/route.ts index 7918814de2..9d117fd357 100644 --- a/src/app/api/tunnels/tailscale/enable/route.ts +++ b/src/app/api/tunnels/tailscale/enable/route.ts @@ -1,5 +1,6 @@ import { NextResponse } from "next/server"; import { enableTailscaleTunnel } from "@/lib/tailscaleTunnel"; +import { toPublicSafeTunnelError } from "@/lib/api/publicSafeTunnelError"; import { parseOptionalJsonBody, requireTailscaleAuth, tailscaleEnableSchema } from "../routeUtils"; export const dynamic = "force-dynamic"; @@ -16,9 +17,11 @@ export async function POST(request: Request) { return NextResponse.json(result); } catch (error) { return NextResponse.json( - { - error: error instanceof Error ? error.message : "Failed to enable Tailscale Funnel", - }, + toPublicSafeTunnelError( + error, + "Failed to enable the Tailscale Funnel.", + "tunnels/tailscale/enable POST" + ), { status: 500 } ); } diff --git a/src/app/api/tunnels/tailscale/install/route.ts b/src/app/api/tunnels/tailscale/install/route.ts index b5f747eeb2..1cecac4b26 100644 --- a/src/app/api/tunnels/tailscale/install/route.ts +++ b/src/app/api/tunnels/tailscale/install/route.ts @@ -1,5 +1,6 @@ import { NextResponse } from "next/server"; import { getTailscaleTunnelStatus, installTailscale } from "@/lib/tailscaleTunnel"; +import { toPublicSafeTunnelError } from "@/lib/api/publicSafeTunnelError"; import { parseOptionalJsonBody, requireTailscaleAuth, tailscaleSudoSchema } from "../routeUtils"; export const dynamic = "force-dynamic"; @@ -31,9 +32,14 @@ export async function POST(request: Request) { status: await getTailscaleTunnelStatus(), }); } catch (error) { - pushEvent("error", { - error: error instanceof Error ? error.message : "Failed to install Tailscale", - }); + pushEvent( + "error", + toPublicSafeTunnelError( + error, + "Failed to install Tailscale.", + "tunnels/tailscale/install POST" + ) + ); } finally { controller.close(); } diff --git a/src/app/api/tunnels/tailscale/login/route.ts b/src/app/api/tunnels/tailscale/login/route.ts index 2606430723..09d9e1ec26 100644 --- a/src/app/api/tunnels/tailscale/login/route.ts +++ b/src/app/api/tunnels/tailscale/login/route.ts @@ -1,5 +1,6 @@ import { NextResponse } from "next/server"; import { startTailscaleLogin } from "@/lib/tailscaleTunnel"; +import { toPublicSafeTunnelError } from "@/lib/api/publicSafeTunnelError"; import { parseOptionalJsonBody, requireTailscaleAuth, tailscaleLoginSchema } from "../routeUtils"; export const dynamic = "force-dynamic"; @@ -16,9 +17,11 @@ export async function POST(request: Request) { return NextResponse.json(result); } catch (error) { return NextResponse.json( - { - error: error instanceof Error ? error.message : "Failed to start Tailscale login", - }, + toPublicSafeTunnelError( + error, + "Failed to start the Tailscale login.", + "tunnels/tailscale/login POST" + ), { status: 500 } ); } diff --git a/src/app/api/tunnels/tailscale/route.ts b/src/app/api/tunnels/tailscale/route.ts index 2adf374ae8..dcba14d660 100644 --- a/src/app/api/tunnels/tailscale/route.ts +++ b/src/app/api/tunnels/tailscale/route.ts @@ -1,5 +1,6 @@ import { NextResponse } from "next/server"; import { getTailscaleTunnelStatus } from "@/lib/tailscaleTunnel"; +import { toPublicSafeTunnelError } from "@/lib/api/publicSafeTunnelError"; import { requireTailscaleAuth } from "./routeUtils"; export const dynamic = "force-dynamic"; @@ -13,9 +14,11 @@ export async function GET(request: Request) { return NextResponse.json(status); } catch (error) { return NextResponse.json( - { - error: error instanceof Error ? error.message : "Failed to load Tailscale status", - }, + toPublicSafeTunnelError( + error, + "Failed to load the Tailscale status.", + "tunnels/tailscale GET" + ), { status: 500 } ); } diff --git a/src/app/api/tunnels/tailscale/routeUtils.ts b/src/app/api/tunnels/tailscale/routeUtils.ts index b6df5118e5..426280a235 100644 --- a/src/app/api/tunnels/tailscale/routeUtils.ts +++ b/src/app/api/tunnels/tailscale/routeUtils.ts @@ -1,7 +1,11 @@ import { NextResponse } from "next/server"; import { z } from "zod"; import { isAuthenticated } from "@/shared/utils/apiAuth"; -import { isValidationFailure, validateBody } from "@/shared/validation/helpers"; +import { + formatValidationMessage, + isValidationFailure, + validateBody, +} from "@/shared/validation/helpers"; export const tailscaleEnableSchema = z.object({ sudoPassword: z.string().optional(), @@ -34,7 +38,15 @@ export async function parseOptionalJsonBody(request: Req const validation = validateBody(schema, rawBody); if (isValidationFailure(validation)) { - return { response: validation.response }; + // validateBody() returns { success, error } — it has no `response` field, so + // the previous `{ response: validation.response }` handed every caller an + // `undefined` response and Next answered with a framework 500 instead of a 400. + return { + response: NextResponse.json( + { error: formatValidationMessage(validation.error) }, + { status: 400 } + ), + }; } return { data: validation.data }; diff --git a/src/app/api/tunnels/tailscale/start-daemon/route.ts b/src/app/api/tunnels/tailscale/start-daemon/route.ts index b3b1208834..7d65bda68f 100644 --- a/src/app/api/tunnels/tailscale/start-daemon/route.ts +++ b/src/app/api/tunnels/tailscale/start-daemon/route.ts @@ -1,5 +1,6 @@ import { NextResponse } from "next/server"; import { getTailscaleTunnelStatus, startTailscaleDaemon } from "@/lib/tailscaleTunnel"; +import { toPublicSafeTunnelError } from "@/lib/api/publicSafeTunnelError"; import { parseOptionalJsonBody, requireTailscaleAuth, tailscaleSudoSchema } from "../routeUtils"; export const dynamic = "force-dynamic"; @@ -19,9 +20,11 @@ export async function POST(request: Request) { }); } catch (error) { return NextResponse.json( - { - error: error instanceof Error ? error.message : "Failed to start the Tailscale daemon", - }, + toPublicSafeTunnelError( + error, + "Failed to start the Tailscale daemon.", + "tunnels/tailscale/start-daemon POST" + ), { status: 500 } ); } diff --git a/src/lib/api/publicSafeTunnelError.ts b/src/lib/api/publicSafeTunnelError.ts new file mode 100644 index 0000000000..b2752f85aa --- /dev/null +++ b/src/lib/api/publicSafeTunnelError.ts @@ -0,0 +1,96 @@ +/** + * Public-safe error bodies for the tunnel and MITM management routes. + * + * Hard Rule #12 forbids returning a raw `err.message` in an HTTP body, and + * `sanitizeErrorMessage()` is the repo's general answer. It is not enough here: + * it only rewrites tokens that look like an absolute path ending in a *source* + * extension (`ts|tsx|js|jsx|mjs|cjs` — see `SOURCE_EXT` in + * open-sse/utils/error.ts), so the three leak shapes these routes actually + * produce all survive it verbatim: + * + * - config/state paths: `ENOENT ... open '/home//.omniroute/data/tunnels.json'` + * - binary paths: `spawn /usr/local/bin/cloudflared ENOENT` + * - Tailscale auth keys: `invalid key tskey-auth-kMn3Qz7RtY-9fVbXsPq2LdWc` + * + * These come from child processes (`cloudflared`, `tailscale`, `tailscaled`, + * `ngrok`) and from filesystem I/O on the operator's home directory, so the raw + * message discloses the host layout, the install location, the OS account name + * and — for Tailscale — a live credential. + * + * Reachability is not uniform. `/api/tunnels/ngrok` (both methods), + * `/api/tunnels/tailscale` and `/api/tunnels/tailscale/check` are NOT in + * `LOCAL_ONLY_API_PREFIXES`, and `GET /api/tunnels/cloudflared` is explicitly + * exempted through `LOCAL_ONLY_API_GET_EXEMPTIONS` (#11531), so those bodies can + * reach a non-loopback caller. The remaining tunnel routes and + * `/api/settings/mitm` are loopback-gated; they are covered here for one + * consistent contract, not because they are remotely reachable. + * + * The contract deliberately does NOT try to scrub the upstream text. It returns + * a fixed operator-facing sentence plus a coarse machine-readable `reason`, and + * logs the real error server-side where the operator can still read it. + */ + +/** Coarse classification a client can branch on without seeing host details. */ +export type PublicSafeTunnelErrorReason = + "not_installed" | "permission_denied" | "already_running" | "timeout" | "network" | "unknown"; + +export interface PublicSafeTunnelErrorBody { + error: string; + reason: PublicSafeTunnelErrorReason; +} + +/** Classify without echoing: only the reason label ever reaches the client. */ +export function classifyTunnelError(error: unknown): PublicSafeTunnelErrorReason { + const raw = (error instanceof Error ? error.message : String(error ?? "")).toLowerCase(); + if (!raw) return "unknown"; + if (raw.includes("enoent") || raw.includes("not found") || raw.includes("not installed")) { + return "not_installed"; + } + if ( + raw.includes("eacces") || + raw.includes("eperm") || + raw.includes("permission denied") || + raw.includes("sudo") || + raw.includes("must be run as root") + ) { + return "permission_denied"; + } + if ( + raw.includes("eaddrinuse") || + raw.includes("already running") || + raw.includes("already in use") + ) { + return "already_running"; + } + if (raw.includes("etimedout") || raw.includes("timed out") || raw.includes("timeout")) { + return "timeout"; + } + if ( + raw.includes("econnrefused") || + raw.includes("econnreset") || + raw.includes("enotfound") || + raw.includes("network") + ) { + return "network"; + } + return "unknown"; +} + +/** + * Build the 500 body for a tunnel/MITM route. + * + * @param error The caught value. Never echoed. + * @param fallback Operator-facing sentence describing what failed. Must be a + * literal owned by the route — never derived from `error`. + * @param context Short route label used only for the server-side log line. + */ +export function toPublicSafeTunnelError( + error: unknown, + fallback: string, + context: string +): PublicSafeTunnelErrorBody { + // The operator still needs the real cause; it belongs in the server log, not + // in a body that may cross a tunnel. + console.error(`[${context}]`, error); + return { error: fallback, reason: classifyTunnelError(error) }; +} diff --git a/tests/unit/tunnel-routes-error-sanitization.test.ts b/tests/unit/tunnel-routes-error-sanitization.test.ts new file mode 100644 index 0000000000..2e5036e9ea --- /dev/null +++ b/tests/unit/tunnel-routes-error-sanitization.test.ts @@ -0,0 +1,268 @@ +/** + * OSS-051 — the tunnel and MITM management routes returned raw `error.message`. + * + * Two independent defects, both in `src/app/api/tunnels/**` and + * `src/app/api/settings/mitm/route.ts`: + * + * 1. Fourteen catch blocks answered with + * `{ error: error instanceof Error ? error.message : "" }`. Those + * errors come from child processes (cloudflared / tailscale / tailscaled / + * ngrok) and from filesystem I/O under the operator's home directory, so the + * body disclosed host layout, binary install paths, the OS account name and — + * for Tailscale — live `tskey-*` credentials. Hard Rule #12 forbids this. + * + * `sanitizeErrorMessage()` alone does not close it: it only rewrites tokens + * that look like an absolute path ending in a *source* extension (SOURCE_EXT + * in open-sse/utils/error.ts), so `.json` state paths, extension-less binary + * paths and `tskey-*` keys all survive it verbatim. The first test below pins + * that, so the reason this module exists stays visible. + * + * 2. `validateBody()` returns `{ success, error }` and has NO `response` field + * (`validatedJsonBody()` is the helper that has one). Three call sites did + * `return validation.response`, so a failed body validation returned + * `undefined` and Next answered with a framework 500 instead of a 400. + * + * Reachability is not uniform, and the tests say so: `/api/tunnels/ngrok`, + * `/api/tunnels/tailscale` and `/api/tunnels/tailscale/check` are not in + * `LOCAL_ONLY_API_PREFIXES`, and `GET /api/tunnels/cloudflared` is exempted via + * `LOCAL_ONLY_API_GET_EXEMPTIONS` (#11531); the rest are loopback-gated. + */ + +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import test from "node:test"; +import type { NextRequest } from "next/server"; + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-tunnel-sanitize-")); +const ORIGINAL_DATA_DIR = process.env.DATA_DIR; +process.env.DATA_DIR = TEST_DATA_DIR; +process.env.API_KEY_SECRET = process.env.API_KEY_SECRET || "tunnel-sanitize-test-secret"; + +const core = await import("../../src/lib/db/core.ts"); +const { sanitizeErrorMessage } = await import("../../open-sse/utils/error.ts"); +const { classifyTunnelError, toPublicSafeTunnelError } = + await import("../../src/lib/api/publicSafeTunnelError.ts"); +const ngrokRoute = await import("../../src/app/api/tunnels/ngrok/route.ts"); +const cloudflaredRoute = await import("../../src/app/api/tunnels/cloudflared/route.ts"); +const tailscaleEnableRoute = await import("../../src/app/api/tunnels/tailscale/enable/route.ts"); + +test.after(() => { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); + if (ORIGINAL_DATA_DIR === undefined) delete process.env.DATA_DIR; + else process.env.DATA_DIR = ORIGINAL_DATA_DIR; +}); + +/** The exact leak shapes these routes produce in the field. */ +const LEAKS = [ + { + label: "config/state path (.json)", + message: + "ENOENT: no such file or directory, open '/home/operator/.omniroute/data/tunnels.json'", + secrets: ["/home/operator", "tunnels.json"], + }, + { + label: "binary path (no extension)", + message: "spawn /usr/local/bin/cloudflared ENOENT", + secrets: ["/usr/local/bin/cloudflared"], + }, + { + label: "tailscale auth key", + message: "tailscale up failed: invalid key tskey-auth-kMn3Qz7RtY-9fVbXsPq2LdWc", + secrets: ["tskey-auth-kMn3Qz7RtY-9fVbXsPq2LdWc"], + }, + { + label: "daemon state path", + message: + "Command failed: /opt/omniroute/bin/tailscaled --state=/var/lib/tailscale/tailscaled.state", + secrets: ["/opt/omniroute/bin/tailscaled", "/var/lib/tailscale"], + }, + { + label: "windows config path", + message: + "listen EADDRINUSE: address already in use 0.0.0.0:41641 (config C:\\Users\\operator\\AppData\\omniroute\\ngrok.yml)", + secrets: ["C:\\Users\\operator", "ngrok.yml"], + }, +] as const; + +/** Run `fn` with console.error captured, so the helper's log does not spam output. */ +async function withSilencedConsoleError(fn: () => T | Promise): Promise<[T, unknown[][]]> { + const original = console.error; + const calls: unknown[][] = []; + console.error = (...args: unknown[]) => { + calls.push(args); + }; + try { + return [await fn(), calls]; + } finally { + console.error = original; + } +} + +// ── Why a dedicated module: sanitizeErrorMessage does not cover these ─────── + +test("sanitizeErrorMessage alone leaves every tunnel leak shape intact", () => { + for (const leak of LEAKS) { + const out = sanitizeErrorMessage(leak.message); + const stillLeaks = leak.secrets.some((s) => out.includes(s)); + assert.ok( + stillLeaks, + `${leak.label}: sanitizeErrorMessage unexpectedly covers this now — if the ` + + `shared sanitizer grew to handle it, simplify publicSafeTunnelError accordingly. Got: ${out}` + ); + } +}); + +// ── The public-safe contract ─────────────────────────────────────────────── + +test("toPublicSafeTunnelError never echoes the upstream message", async () => { + for (const leak of LEAKS) { + const [body] = await withSilencedConsoleError(() => + toPublicSafeTunnelError(new Error(leak.message), "Failed to update the tunnel.", "test") + ); + assert.equal(body.error, "Failed to update the tunnel.", `${leak.label}: fallback replaced`); + for (const secret of leak.secrets) { + assert.ok(!body.error.includes(secret), `${leak.label}: leaked ${secret}`); + assert.ok( + !JSON.stringify(body).includes(secret), + `${leak.label}: leaked ${secret} elsewhere in the body` + ); + } + assert.equal(typeof body.reason, "string"); + } +}); + +test("toPublicSafeTunnelError logs the real error server-side", async () => { + const [, calls] = await withSilencedConsoleError(() => + toPublicSafeTunnelError(new Error("spawn /usr/local/bin/cloudflared ENOENT"), "nope", "ctx") + ); + assert.equal(calls.length, 1, "operator must still get the real cause in the server log"); + assert.equal(calls[0][0], "[ctx]"); +}); + +test("classifyTunnelError maps causes without echoing them", () => { + assert.equal(classifyTunnelError(new Error("spawn cloudflared ENOENT")), "not_installed"); + assert.equal(classifyTunnelError(new Error("EACCES: permission denied")), "permission_denied"); + assert.equal(classifyTunnelError(new Error("listen EADDRINUSE")), "already_running"); + assert.equal(classifyTunnelError(new Error("connect ETIMEDOUT")), "timeout"); + assert.equal(classifyTunnelError(new Error("connect ECONNREFUSED")), "network"); + assert.equal(classifyTunnelError(new Error("something else entirely")), "unknown"); + assert.equal(classifyTunnelError(undefined), "unknown"); +}); + +// ── Defect 1, end to end on a remotely reachable route ───────────────────── + +function makeRequest(url: string, init?: RequestInit): NextRequest { + return new Request(url, init) as unknown as NextRequest; +} + +test("GET /api/tunnels/ngrok does not leak a host path in its 500 body", async () => { + // getNgrokTunnelStatus() reads globalThis.__ngrokListener and then calls + // getTunnelApiUrl(currentUrl) OUTSIDE its try/catch, so a listener whose url() + // yields an object with a throwing `replace` reproduces a real 500 here. + const LEAK = "/home/operator/.omniroute/data/tunnels.json"; + const g = globalThis as unknown as { __ngrokListener?: unknown }; + const previous = g.__ngrokListener; + g.__ngrokListener = { + url: () => ({ + replace: () => { + throw new Error(`ENOENT: no such file or directory, open '${LEAK}'`); + }, + }), + }; + + try { + const [res] = await withSilencedConsoleError(() => + ngrokRoute.GET(makeRequest("http://localhost/api/tunnels/ngrok")) + ); + assert.equal(res.status, 500); + const body = (await res.json()) as { error?: unknown; reason?: unknown }; + assert.equal(typeof body.error, "string", "dashboard reads data.error as a string"); + const text = JSON.stringify(body); + assert.ok(!text.includes(LEAK), `body leaked the state path: ${text}`); + assert.ok(!text.includes("/home/operator"), `body leaked the home directory: ${text}`); + assert.equal(body.reason, "not_installed"); + } finally { + if (previous === undefined) delete g.__ngrokListener; + else g.__ngrokListener = previous; + } +}); + +// ── Defect 2: validateBody has no `response` field ───────────────────────── + +test("POST /api/tunnels/ngrok answers 400 (not a framework 500) on an invalid body", async () => { + const res = await ngrokRoute.POST( + makeRequest("http://localhost/api/tunnels/ngrok", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ action: "not-a-valid-action" }), + }) + ); + assert.ok(res, "handler must return a Response, not undefined"); + assert.equal(res.status, 400); + const body = (await res.json()) as { error?: unknown }; + assert.equal(typeof body.error, "string"); + assert.ok((body.error as string).length > 0); +}); + +test("POST /api/tunnels/cloudflared answers 400 (not a framework 500) on an invalid body", async () => { + const res = await cloudflaredRoute.POST( + makeRequest("http://localhost/api/tunnels/cloudflared", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ action: "not-a-valid-action" }), + }) + ); + assert.ok(res, "handler must return a Response, not undefined"); + assert.equal(res.status, 400); + assert.equal(typeof ((await res.json()) as { error?: unknown }).error, "string"); +}); + +test("POST /api/tunnels/tailscale/enable answers 400 (not a framework 500) on an out-of-range port", async () => { + const res = await tailscaleEnableRoute.POST( + makeRequest("http://localhost/api/tunnels/tailscale/enable", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ port: 999999 }), + }) + ); + assert.ok(res, "handler must return a Response, not undefined"); + assert.equal(res.status, 400); + const body = (await res.json()) as { error?: unknown }; + assert.equal(typeof body.error, "string"); + assert.match(body.error as string, /port/i, "the 400 should name the offending field"); +}); + +// ── Regression sweep over the whole surface this PR covers ──────────────── + +test("no tunnel or MITM route echoes a raw error.message any more", () => { + const roots = ["src/app/api/tunnels", "src/app/api/settings/mitm"]; + const offenders: string[] = []; + + const walk = (dir: string) => { + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) { + walk(full); + continue; + } + if (!entry.name.endsWith(".ts")) continue; + for (const [i, line] of fs.readFileSync(full, "utf8").split("\n").entries()) { + // The comments this PR adds mention the old pattern by name; only flag code. + if (line.trimStart().startsWith("//")) continue; + if (/\b\w+ instanceof Error \? \w+\.message\b/.test(line)) { + offenders.push(`${full}:${i + 1}`); + } + } + } + }; + + for (const root of roots) walk(path.resolve(process.cwd(), root)); + assert.deepEqual( + offenders, + [], + `Hard Rule #12: these lines put a raw error.message in a response body:\n${offenders.join("\n")}` + ); +});