diff --git a/bin/cli/commands/serve.mjs b/bin/cli/commands/serve.mjs index 456e2e6f77..b58271e833 100644 --- a/bin/cli/commands/serve.mjs +++ b/bin/cli/commands/serve.mjs @@ -12,7 +12,7 @@ import { isFatalInstrumentationHookFailure, formatAndroidInstrumentationFailureHint, } from "../utils/ensureAndroidCacheDir.mjs"; -import { resolveServerHost } from "../utils/serverHost.mjs"; +import { resolveServerHost, resolveExposureWarning } from "../utils/serverHost.mjs"; import { resolveMaxOldSpaceMb, calibrateHeapFallbackMb, @@ -162,6 +162,15 @@ export async function runServe(opts = {}) { `); } + // GHSA-wmgv-ph3p-rv57: the default posture (all interfaces + no API key) is a + // deliberate local-first choice, but it must be loud at startup — an operator + // on an untrusted network learns the two escape hatches here, not after a + // surprise quota bill. + const exposureWarning = resolveExposureWarning(); + if (exposureWarning) { + console.warn(`\x1b[33m ⚠ ${exposureWarning}\x1b[0m\n`); + } + const serverWsJs = join(APP_DIR, "server-ws.mjs"); const serverJs = existsSync(serverWsJs) ? serverWsJs : join(APP_DIR, "server.js"); diff --git a/bin/cli/utils/serverHost.mjs b/bin/cli/utils/serverHost.mjs index a64a88d2a6..a13082612f 100644 --- a/bin/cli/utils/serverHost.mjs +++ b/bin/cli/utils/serverHost.mjs @@ -24,3 +24,34 @@ export function resolveServerHost( } return "0.0.0.0"; } + +const LOOPBACK_HOSTS = new Set(["127.0.0.1", "localhost", "::1", "[::1]"]); + +/** + * Boot-time exposure warning (GHSA-wmgv-ph3p-rv57): the shipped default binds + * all interfaces while the inference plane requires no credentials, so any + * LAN peer can spend the operator's quota. That local-first posture is a + * deliberate, documented default — but it must be LOUD at startup so an + * operator who never read the docs still learns the two escape hatches. + * + * Returns the warning text when the server will listen on a non-loopback + * interface with no API-key requirement, or null when the exposure is closed. + * + * @param {NodeJS.ProcessEnv} [env] + * @param {string} [host] + * @returns {string | null} + */ +export function resolveExposureWarning(env = process.env, host = resolveServerHost(env)) { + if (LOOPBACK_HOSTS.has(host)) return null; + const requireKey = String(env.REQUIRE_API_KEY || "") + .trim() + .toLowerCase(); + if (requireKey === "true" || requireKey === "1" || requireKey === "yes") return null; + return ( + `SECURITY: listening on ${host} with NO API-key requirement — the inference ` + + `plane (/v1/*) is reachable by ANY device that can route to this host, and ` + + `requests are billed to your configured providers. This local-first default ` + + `is intentional, but on an untrusted network either set REQUIRE_API_KEY=true ` + + `or bind loopback with OMNIROUTE_SERVER_HOST=127.0.0.1.` + ); +} diff --git a/open-sse/handlers/search.ts b/open-sse/handlers/search.ts index d5cbc5fa38..858ea6f68a 100644 --- a/open-sse/handlers/search.ts +++ b/open-sse/handlers/search.ts @@ -31,6 +31,7 @@ import * as xSearch from "./search/xSearch.ts"; import { freeWebSearch } from "../services/freeWebSearch.ts"; import { saveCallLog } from "@/lib/usageDb"; import { safeOutboundFetch } from "@/shared/network/safeOutboundFetch"; +import { parseAndValidateNonMetadataUrl } from "@/shared/network/outboundUrlGuard"; import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { StreamableHTTPClientTransport } from "@modelcontextprotocol/sdk/client/streamableHttp.js"; import { z } from "zod"; @@ -313,9 +314,23 @@ function getProviderSettingString( return undefined; } -function resolveSearchBaseUrl(config: SearchProviderConfig, params: SearchRequestParams): string { +export function resolveSearchBaseUrl( + config: SearchProviderConfig, + params: SearchRequestParams +): string { const override = getProviderSettingString(params, "baseUrl"); - return (override || config.baseUrl).replace(/\/+$/, ""); + if (override) { + // GHSA-j7j4-g9qc-q69c: the override is client-controlled (provider_options / + // providerSpecificData) and flows into a plain fetch() sink — validate it + // before any builder uses it as the server-side fetch target. Mode is + // block-metadata (NOT public-only): the primary searxng use case is a + // self-hosted instance on loopback/LAN, so private hosts keep working, + // while cloud-metadata endpoints (IMDS credential theft) are rejected. + // The catalog's own config.baseUrl is operator config and stays untouched. + parseAndValidateNonMetadataUrl(override); + return override.replace(/\/+$/, ""); + } + return config.baseUrl.replace(/\/+$/, ""); } function toSearchPageNumber(offset: number | undefined, maxResults: number): number | undefined { diff --git a/src/app/a2a/route.ts b/src/app/a2a/route.ts index af7d93a2e5..dfe3fc73a7 100644 --- a/src/app/a2a/route.ts +++ b/src/app/a2a/route.ts @@ -10,15 +10,13 @@ * Auth: Bearer token via Authorization header */ -import { timingSafeEqual } from "node:crypto"; import { NextRequest, NextResponse } from "next/server"; import { getTaskManager } from "@/lib/a2a/taskManager"; import { logRoutingDecision } from "@/lib/a2a/routingLogger"; import { createA2AStream, SSE_HEADERS } from "@/lib/a2a/streaming"; import { A2A_SKILL_HANDLERS, executeA2ATaskWithState } from "@/lib/a2a/taskExecution"; import { getSettings } from "@/lib/db/settings"; -import { isRequireApiKeyEnabled } from "@/shared/utils/featureFlags"; -import { extractApiKey, isValidApiKey } from "@/sse/services/auth"; +import { authenticateA2ARequest, resolveA2AOwner } from "@/lib/a2a/authenticate"; // ============ A2A v1.0 ↔ v0.3 compatibility layer ============ // A2A 1.0 renamed the JSON-RPC methods (message/send → SendMessage, @@ -55,7 +53,7 @@ function buildV1Task( ? result.artifacts .map((a) => a && typeof a === "object" && typeof (a as { content?: unknown }).content === "string" - ? ((a as { content: string }).content) + ? (a as { content: string }).content : "" ) .filter((s) => s.length > 0) @@ -124,39 +122,13 @@ function toMessageArray(raw: unknown): A2AMessage[] | null { // ============ Auth ============ -/** - * Constant-time comparison of the presented bearer token against the configured - * key. A plain `===` short-circuits on the first differing byte, leaking the - * length of the shared prefix through response timing; `timingSafeEqual` does - * not. It requires equal-length buffers, so mismatched lengths are rejected up - * front (the length itself is not secret). - */ -function tokensMatch(provided: string, expected: string): boolean { - const a = Buffer.from(provided); - const b = Buffer.from(expected); - if (a.length !== b.length) return false; - return timingSafeEqual(a, b); -} - async function authenticate(req: NextRequest): Promise { // /a2a is outside the authz proxy matcher, so the REQUIRE_API_KEY posture the // pipeline enforces for /v1 never ran here — the route accepted every caller // whenever OMNIROUTE_API_KEY was unset, which is the shipped default - // (GHSA-v54m-6rm3-p565). Apply the same posture directly: when a client key is - // required, demand a valid OmniRoute key; otherwise honor the legacy explicit - // A2A key; otherwise stay keyless (the same local-first default as /v1). - const apiKey = extractApiKey(req); - if (isRequireApiKeyEnabled()) { - return apiKey ? await isValidApiKey(apiKey) : false; - } - - const configuredKey = process.env.OMNIROUTE_API_KEY; - if (configuredKey) { - return apiKey ? tokensMatch(apiKey, configuredKey) : false; - } - - // No API key required and none configured — allow (keyless local-first). - return true; + // (GHSA-v54m-6rm3-p565). The shared helper applies the same posture on both + // the JSON-RPC and the REST task surfaces (GHSA-jcm5-6wpp-wjj8). + return authenticateA2ARequest(req); } // ============ JSON-RPC Helpers ============ @@ -213,6 +185,9 @@ export async function POST(req: NextRequest) { if (disabledResponse) return disabledResponse; const tm = getTaskManager(); + // GHSA-jcm5-6wpp-wjj8: scope every task read/mutation below to the caller's + // owner id (hashed API key; undefined under the keyless local-first posture). + const callerOwner = resolveA2AOwner(req); // A2A 1.0 method-name compatibility (SendMessage → message/send, etc.) const isV1Method = method in V1_METHOD_ALIASES; @@ -236,7 +211,7 @@ export async function POST(req: NextRequest) { return jsonRpcError(id, -32601, `Unknown skill: ${skill}`); } - const task = tm.createTask({ skill, messages, metadata: params?.metadata }); + const task = tm.createTask({ skill, messages, metadata: params?.metadata }, callerOwner); try { tm.updateTask(task.id, "working"); const result = await handler(task); @@ -302,7 +277,7 @@ export async function POST(req: NextRequest) { return jsonRpcError(id, -32601, `Unknown skill: ${skill}`); } - const task = tm.createTask({ skill, messages, metadata: params?.metadata }); + const task = tm.createTask({ skill, messages, metadata: params?.metadata }, callerOwner); tm.updateTask(task.id, "working"); const stream = createA2AStream( @@ -323,7 +298,7 @@ export async function POST(req: NextRequest) { const taskId = params?.taskId || params?.id; if (!taskId) return jsonRpcError(id, -32602, "Invalid params: taskId required"); - const task = tm.getTask(taskId); + const task = tm.getTask(taskId, callerOwner); if (!task) return jsonRpcError(id, -32601, `Task not found: ${taskId}`); return jsonRpcResult(id, { task }); @@ -335,7 +310,7 @@ export async function POST(req: NextRequest) { if (!taskId) return jsonRpcError(id, -32602, "Invalid params: taskId required"); try { - const task = tm.cancelTask(taskId); + const task = tm.cancelTask(taskId, callerOwner); return jsonRpcResult(id, { task: { id: task.id, state: task.state } }); } catch (err) { const msg = err instanceof Error ? err.message : String(err); diff --git a/src/app/api/a2a/_auth.ts b/src/app/api/a2a/_auth.ts new file mode 100644 index 0000000000..2ec286db91 --- /dev/null +++ b/src/app/api/a2a/_auth.ts @@ -0,0 +1,51 @@ +/** + * Shared authorization for the REST A2A task routes (GHSA-jcm5-6wpp-wjj8). + * + * Dual audience: the dashboard calls these routes with a management session, + * A2A clients with an inference API key. Posture matrix: + * + * - REQUIRE_API_KEY=true: a valid OmniRoute key is mandatory (the same + * posture the /v1 inference plane enforces); a management session also + * passes (dashboard), via alwaysRequireAuth so requireLogin=false cannot + * bypass it. + * - otherwise + requireLogin=true: management session, or a valid key. + * - otherwise + requireLogin=false (local-first default): open, by design. + * + * Callers authenticated by key are owner-scoped — another principal's tasks + * answer as if they did not exist. Management/operator view sees all tasks. + */ + +import { requireManagementAuth } from "@/lib/api/requireManagementAuth"; +import { extractApiKey, isValidApiKey } from "@/sse/services/auth"; +import { isRequireApiKeyEnabled } from "@/shared/utils/featureFlags"; +import { resolveA2AOwner } from "@/lib/a2a/authenticate"; + +export interface A2ARestAuth { + /** Owner scope for task reads/mutations; undefined = operator view (all tasks). */ + owner: string | undefined; +} + +/** + * NOTE: the failure branch is whatever requireManagementAuth returns — today a + * plain `Response` from createErrorResponse(), NOT a NextResponse. Callers must + * test with `instanceof Response` (NextResponse extends Response), never + * `instanceof NextResponse`, or the 401 silently falls through to the handler. + */ +export async function authorizeA2ATaskRoute(request: Request): Promise { + const apiKey = extractApiKey(request); + + if (isRequireApiKeyEnabled()) { + if (apiKey && (await isValidApiKey(apiKey))) return { owner: resolveA2AOwner(request) }; + const managementError = await requireManagementAuth(request, { + invalidApiKeyStatus: 401, + alwaysRequireAuth: true, + }); + if (managementError === null) return { owner: undefined }; + return managementError; + } + + const managementError = await requireManagementAuth(request, { invalidApiKeyStatus: 401 }); + if (managementError === null) return { owner: undefined }; + if (apiKey && (await isValidApiKey(apiKey))) return { owner: resolveA2AOwner(request) }; + return managementError; +} diff --git a/src/app/api/a2a/tasks/[id]/cancel/route.ts b/src/app/api/a2a/tasks/[id]/cancel/route.ts index 9919626f39..bc3558b06e 100644 --- a/src/app/api/a2a/tasks/[id]/cancel/route.ts +++ b/src/app/api/a2a/tasks/[id]/cancel/route.ts @@ -1,14 +1,23 @@ import { NextResponse } from "next/server"; import { getTaskManager } from "@/lib/a2a/taskManager"; +import { authorizeA2ATaskRoute } from "@/app/api/a2a/_auth"; +import { sanitizeErrorMessage } from "@omniroute/open-sse/utils/error"; -export async function POST(_request: Request, { params }: { params: Promise<{ id: string }> }) { +export async function POST(request: Request, { params }: { params: Promise<{ id: string }> }) { + // GHSA-jcm5-6wpp-wjj8: this route had no auth call at all. The owner check + // happens inside cancelTask: another principal's task throws the same + // "not found" a missing one would (no existence oracle). + const auth = await authorizeA2ATaskRoute(request); + if (auth instanceof Response) return auth; try { const { id } = await params; const tm = getTaskManager(); - const task = tm.cancelTask(id); + const task = tm.cancelTask(id, auth.owner); return NextResponse.json({ task: { id: task.id, state: task.state } }); } catch (error) { - const message = error instanceof Error ? error.message : "Failed to cancel A2A task"; + const message = sanitizeErrorMessage( + error instanceof Error ? error.message : "Failed to cancel A2A task" + ); const status = message.includes("not found") ? 404 : 400; return NextResponse.json({ error: message }, { status }); } diff --git a/src/app/api/a2a/tasks/[id]/route.ts b/src/app/api/a2a/tasks/[id]/route.ts index ae3906171e..2d5c1bf0c3 100644 --- a/src/app/api/a2a/tasks/[id]/route.ts +++ b/src/app/api/a2a/tasks/[id]/route.ts @@ -1,17 +1,30 @@ import { NextResponse } from "next/server"; import { getTaskManager } from "@/lib/a2a/taskManager"; +import { authorizeA2ATaskRoute } from "@/app/api/a2a/_auth"; +import { sanitizeErrorMessage } from "@omniroute/open-sse/utils/error"; -export async function GET(_request: Request, { params }: { params: Promise<{ id: string }> }) { +export async function GET(request: Request, { params }: { params: Promise<{ id: string }> }) { + // GHSA-jcm5-6wpp-wjj8: this route had no auth call at all — open regardless + // of configuration. Another principal's task answers 404, same as a missing + // one, so an IDOR probe cannot tell the two apart. + const auth = await authorizeA2ATaskRoute(request); + if (auth instanceof Response) return auth; try { const { id } = await params; const tm = getTaskManager(); - const task = tm.getTask(id); + const task = tm.getTask(id, auth.owner); if (!task) { return NextResponse.json({ error: `Task not found: ${id}` }, { status: 404 }); } return NextResponse.json({ task }); } catch (error) { - const message = error instanceof Error ? error.message : "Failed to load A2A task"; - return NextResponse.json({ error: message }, { status: 500 }); + return NextResponse.json( + { + error: sanitizeErrorMessage( + error instanceof Error ? error.message : "Failed to load A2A task" + ), + }, + { status: 500 } + ); } } diff --git a/src/app/api/a2a/tasks/route.ts b/src/app/api/a2a/tasks/route.ts index ddd1ad60f1..18353dfa7d 100644 --- a/src/app/api/a2a/tasks/route.ts +++ b/src/app/api/a2a/tasks/route.ts @@ -3,6 +3,7 @@ import { NextResponse } from "next/server"; import { z } from "zod"; import { getTaskManager, type TaskState } from "@/lib/a2a/taskManager"; +import { authorizeA2ATaskRoute } from "@/app/api/a2a/_auth"; import { createConductorTask } from "@/lib/conductor/hubProxy"; import { getSettings } from "@/lib/db/settings"; @@ -22,6 +23,11 @@ function parseIntParam(value: string | null, fallback: number): number { } export async function GET(request: Request) { + // GHSA-jcm5-6wpp-wjj8: the list route had no auth call at all. Management + // (or the keyless posture) sees every task; a bare API key must be valid + // and is owner-scoped. + const auth = await authorizeA2ATaskRoute(request); + if (auth instanceof Response) return auth; try { const { searchParams } = new URL(request.url); const stateParam = searchParams.get("state"); @@ -36,7 +42,7 @@ export async function GET(request: Request) { const tm = getTaskManager(); const total = tm.countTasks({ state, skill }); - const tasks = tm.listTasks({ state, skill, limit, offset }); + const tasks = tm.listTasks({ state, skill, limit, offset }, auth.owner); return NextResponse.json({ tasks, @@ -104,7 +110,10 @@ export function authenticateA2A(request: Request): boolean { */ export async function POST(request: Request) { if (!authenticateA2A(request)) { - return NextResponse.json({ error: "Unauthorized: missing or invalid API key" }, { status: 401 }); + return NextResponse.json( + { error: "Unauthorized: missing or invalid API key" }, + { status: 401 } + ); } const settings = await getSettings(); if (settings.a2aEnabled !== true) { @@ -122,12 +131,18 @@ export async function POST(request: Request) { } const parsed = delegationSchema.safeParse(raw); if (!parsed.success) { - return NextResponse.json({ error: "Invalid A2A task: provide messages[] (and metadata.conductor)" }, { status: 400 }); + return NextResponse.json( + { error: "Invalid A2A task: provide messages[] (and metadata.conductor)" }, + { status: 400 } + ); } const { skill, messages, metadata } = parsed.data; if (skill !== "conductor" && !skill.startsWith("conductor-cli-")) { return NextResponse.json( - { error: "Only Conductor fleet skills are delegable here (conductor / conductor-cli-)" }, + { + error: + "Only Conductor fleet skills are delegable here (conductor / conductor-cli-)", + }, { status: 400 } ); } @@ -138,7 +153,9 @@ export async function POST(request: Request) { { status: 400 } ); } - const prompt = [...messages].reverse().find((m) => m.role === "user")?.content ?? messages[messages.length - 1].content; + const prompt = + [...messages].reverse().find((m) => m.role === "user")?.content ?? + messages[messages.length - 1].content; const created = await createConductorTask({ repoUrl: conductor.repo.url, diff --git a/src/lib/a2a/authenticate.ts b/src/lib/a2a/authenticate.ts new file mode 100644 index 0000000000..b57d4082cc --- /dev/null +++ b/src/lib/a2a/authenticate.ts @@ -0,0 +1,53 @@ +/** + * Shared A2A authentication + caller-owner resolution (GHSA-jcm5-6wpp-wjj8). + * + * The JSON-RPC router (/a2a) grew its own authenticate() for GHSA-v54m, but + * the REST task routes under /api/a2a/tasks/ had no auth call at all. Both + * surfaces now share this single implementation so they cannot drift again: + * same REQUIRE_API_KEY posture as /v1, same keyless local-first default, and + * a stable owner id (hashed API key) used to scope task visibility. + */ + +import { createHash, timingSafeEqual } from "crypto"; +import type { NextRequest } from "next/server"; +import { extractApiKey, isValidApiKey } from "@/sse/services/auth"; +import { isRequireApiKeyEnabled } from "@/shared/utils/featureFlags"; + +function tokensMatch(provided: string, expected: string): boolean { + const a = Buffer.from(provided); + const b = Buffer.from(expected); + if (a.length !== b.length) return false; + return timingSafeEqual(a, b); +} + +/** + * Whether the request may use the A2A surface at all. Mirrors the JSON-RPC + * posture: when a client key is required, demand a valid OmniRoute key; + * otherwise honor the legacy explicit A2A key; otherwise stay keyless (the + * same local-first default as /v1). + */ +export async function authenticateA2ARequest(req: NextRequest | Request): Promise { + const apiKey = extractApiKey(req as NextRequest); + if (isRequireApiKeyEnabled()) { + return apiKey ? await isValidApiKey(apiKey) : false; + } + + const configuredKey = process.env.OMNIROUTE_API_KEY; + if (configuredKey) { + return apiKey ? tokensMatch(apiKey, configuredKey) : false; + } + + // No API key required and none configured — allow (keyless local-first). + return true; +} + +/** + * Owner id for task scoping (GHSA-jcm5-6wpp-wjj8): a stable hash of the + * caller's API key, or `undefined` when the call carries no key (keyless + * posture — ownerless tasks stay visible to everyone, by design). + */ +export function resolveA2AOwner(req: NextRequest | Request): string | undefined { + const apiKey = extractApiKey(req as NextRequest); + if (!apiKey) return undefined; + return createHash("sha256").update(apiKey).digest("hex").slice(0, 32); +} diff --git a/src/lib/a2a/taskManager.ts b/src/lib/a2a/taskManager.ts index 390bcda03d..a21ac57207 100644 --- a/src/lib/a2a/taskManager.ts +++ b/src/lib/a2a/taskManager.ts @@ -45,6 +45,13 @@ export interface A2ATask { createdAt: string; updatedAt: string; expiresAt: string; + /** + * GHSA-jcm5-6wpp-wjj8: principal that created the task (hashed API key). + * `undefined` = created under the keyless local-first posture — such tasks + * stay visible to every caller, matching the pre-owner behavior. Tasks WITH + * an owner are only returned/cancelled/listed for the same owner. + */ + owner?: string; } export interface TaskListFilter { @@ -91,7 +98,7 @@ export class A2ATaskManager { } } - createTask(input: TaskInput): A2ATask { + createTask(input: TaskInput, owner?: string): A2ATask { const now = new Date(); const task: A2ATask = { id: randomUUID(), @@ -104,19 +111,31 @@ export class A2ATaskManager { createdAt: now.toISOString(), updatedAt: now.toISOString(), expiresAt: new Date(now.getTime() + this.ttlMs).toISOString(), + ...(owner !== undefined ? { owner } : {}), }; this.tasks.set(task.id, task); return task; } - getTask(taskId: string): A2ATask | undefined { + /** + * Owner scoping (GHSA-jcm5-6wpp-wjj8): a task carrying an owner is visible + * only to that owner. Ownerless tasks (keyless posture, or created before + * this field existed) stay visible to everyone — no behavior change there. + */ + private isVisibleTo(task: A2ATask, owner?: string): boolean { + return task.owner === undefined || task.owner === owner; + } + + getTask(taskId: string, owner?: string): A2ATask | undefined { const task = this.tasks.get(taskId); if (task && new Date(task.expiresAt) < new Date()) { if (task.state === "submitted" || task.state === "working") { this.updateTask(taskId, "failed", undefined, "Task expired"); } } - return this.tasks.get(taskId); + const current = this.tasks.get(taskId); + if (!current || !this.isVisibleTo(current, owner)) return undefined; + return current; } updateTask( @@ -142,7 +161,15 @@ export class A2ATaskManager { return task; } - cancelTask(taskId: string): A2ATask { + cancelTask(taskId: string, owner?: string): A2ATask { + // Owner check BEFORE the mutation (GHSA-jcm5-6wpp-wjj8): a caller must not + // cancel another principal's task by id. Uses the same not-found error as + // a missing task so an IDOR probe cannot distinguish "exists but not + // yours" from "does not exist". + const task = this.tasks.get(taskId); + if (!task || !this.isVisibleTo(task, owner)) { + throw new Error(`Task ${taskId} not found`); + } return this.updateTask(taskId, "cancelled", undefined, "Cancelled by client"); } @@ -153,8 +180,11 @@ export class A2ATaskManager { return tasks.length; } - listTasks(filter?: TaskListFilter): A2ATask[] { + listTasks(filter?: TaskListFilter, owner?: string): A2ATask[] { let tasks = [...this.tasks.values()]; + // GHSA-jcm5-6wpp-wjj8: when an owner scope is supplied, owned tasks of + // other principals are hidden; ownerless tasks remain visible (posture). + if (owner !== undefined) tasks = tasks.filter((t) => this.isVisibleTo(t, owner)); if (filter?.state) tasks = tasks.filter((t) => t.state === filter.state); if (filter?.skill) tasks = tasks.filter((t) => t.skill === filter.skill); tasks.sort((a, b) => new Date(b.createdAt).getTime() - new Date(a.createdAt).getTime()); diff --git a/src/server/authz/routeGuard.ts b/src/server/authz/routeGuard.ts index 7c61d24545..c5cbe01501 100644 --- a/src/server/authz/routeGuard.ts +++ b/src/server/authz/routeGuard.ts @@ -43,6 +43,8 @@ export const LOCAL_ONLY_API_PREFIXES: ReadonlyArray = [ "/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 "/api/tools/agent-bridge/", // AgentBridge: spawns MITM server + DNS edits (Hard Rules #15 + #17) + "/api/settings/mitm", // "Enable MITM" flow: installs a system-wide trusted root CA (security add-trusted-cert / certutil / update-ca-certificates) and writes /etc/hosts DNS overrides via src/mitm/* — host-level TLS interception. Was MANAGEMENT-only, so requireLogin=false left it remotely reachable (GHSA-x7vm-hp44-9p79, Hard Rules #15 + #17). Same tier as /api/tools/agent-bridge/. + "/api/cli-tools/antigravity-mitm", // Antigravity MITM enable flow: same privileged CA-trust + DNS surface as /api/settings/mitm (GHSA-x7vm-hp44-9p79, Hard Rules #15 + #17). Covers the /alias child route by prefix. "/api/tools/traffic-inspector/", // Traffic Inspector: http-proxy listener + system proxy (Hard Rules #15 + #17) "/api/issue-agent/", // Issue Agent: recorded/local triage executor surface; keep loopback/LAN until sandbox + audit hardening is complete "/api/plugins/", // plugins: load/execute via worker_threads + child_process (Hard Rules #15 + #17) @@ -126,6 +128,12 @@ export const ALWAYS_PROTECTED_API_PATHS: ReadonlyArray = [ // /api/settings/database already does. isAlwaysProtectedPath matches on a path // boundary, so this covers export, exportAll and import. (GHSA-mghq-58h3-qcqj) "/api/db-backups", + // Legacy siblings of /api/db-backups left out of the mghq fix: export-json + // dumps every stored credential and import-json irreversibly replaces + // settings/connections, and both handlers only gate on isAuthRequired() — + // which is false under requireLogin=false. (GHSA-v7g9-7f55-5g46) + "/api/settings/export-json", + "/api/settings/import-json", ]; export function isLoopbackHost(hostHeader: string | null): boolean { diff --git a/src/shared/constants/spawnCapablePrefixes.ts b/src/shared/constants/spawnCapablePrefixes.ts index 20787d7d2f..3a1a05a881 100644 --- a/src/shared/constants/spawnCapablePrefixes.ts +++ b/src/shared/constants/spawnCapablePrefixes.ts @@ -28,6 +28,8 @@ export const SPAWN_CAPABLE_PREFIXES: ReadonlyArray = [ "/api/cli-tools/qwen-settings", // GET probes the Qwen Code binary; the route also mutates local ~/.qwen files "/api/services/", // T-10: can run npm install + spawn node processes "/api/tools/agent-bridge/", // start/stop MITM server + DNS edits (Hard Rules #15 + #17) + "/api/settings/mitm", // installs a system trusted root CA + /etc/hosts DNS overrides via src/mitm/* — must never be whitelistable via manage-scope bypass (GHSA-x7vm-hp44-9p79, Hard Rules #15 + #17) + "/api/cli-tools/antigravity-mitm", // same privileged CA-trust + DNS surface as /api/settings/mitm (GHSA-x7vm-hp44-9p79, Hard Rules #15 + #17) "/api/tools/traffic-inspector/", // http-proxy listener + system proxy (Hard Rules #15 + #17) "/api/plugins/", // plugins: load/execute via worker_threads + child_process (Hard Rules #15 + #17) "/api/local/", // T-12: 1-click local service launchers (Redis today) — must never be whitelistable via manage-scope bypass (Hard Rules #15 + #17) diff --git a/tests/unit/a2a-task-owner-idor.test.ts b/tests/unit/a2a-task-owner-idor.test.ts new file mode 100644 index 0000000000..365744ed33 --- /dev/null +++ b/tests/unit/a2a-task-owner-idor.test.ts @@ -0,0 +1,136 @@ +/** + * GHSA-jcm5-6wpp-wjj8 — A2A task IDOR + unauthenticated REST task routes. + * + * Two gaps closed here: + * 1. The REST routes /api/a2a/tasks/[id] and /api/a2a/tasks/[id]/cancel had + * NO auth call at all — open regardless of configuration. They now share + * the JSON-RPC surface's authentication (REQUIRE_API_KEY posture). + * 2. Tasks lived in an owner-less Map: any caller could read/cancel any + * task by id. Tasks now bind to an owner (hashed API key) at creation and + * reads/cancels/lists are owner-scoped. Ownerless tasks (keyless + * local-first posture) stay visible to everyone — by design. + * + * Run with: + * node --import tsx/esm --test tests/unit/a2a-task-owner-idor.test.ts + */ + +import { describe, it, after } from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omni-a2a-idor-")); +process.env.DATA_DIR = TEST_DATA_DIR; +process.env.API_KEY_SECRET = process.env.API_KEY_SECRET || "a2a-idor-test-secret"; +process.env.OMNIROUTE_DISABLE_REDIS_AUTH_CACHE = "1"; + +const core = await import("../../src/lib/db/core.ts"); +const apiKeysDb = await import("../../src/lib/db/apiKeys.ts"); +const { A2ATaskManager, getTaskManager } = await import("../../src/lib/a2a/taskManager.ts"); +const { resolveA2AOwner } = await import("../../src/lib/a2a/authenticate.ts"); +const restGet = await import("../../src/app/api/a2a/tasks/[id]/route.ts"); + +const ORIGINAL_REQUIRE = process.env.REQUIRE_API_KEY; + +after(() => { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); + if (ORIGINAL_REQUIRE === undefined) delete process.env.REQUIRE_API_KEY; + else process.env.REQUIRE_API_KEY = ORIGINAL_REQUIRE; +}); + +function makeManager() { + const tm = new A2ATaskManager(5); + // Prevent the per-instance cleanup interval from keeping the process alive. + clearInterval((tm as unknown as { cleanupInterval: NodeJS.Timeout }).cleanupInterval); + return tm; +} + +describe("A2ATaskManager — owner scoping (GHSA-jcm5)", () => { + it("another principal cannot READ an owned task (same undefined as missing)", () => { + const tm = makeManager(); + const task = tm.createTask({ skill: "smart-routing", messages: [] }, "owner-a"); + assert.equal(tm.getTask(task.id, "owner-a")?.id, task.id, "the owner still reads it"); + assert.equal(tm.getTask(task.id, "owner-b"), undefined, "another owner gets undefined"); + }); + + it("another principal cannot CANCEL an owned task (not-found error, no existence oracle)", () => { + const tm = makeManager(); + const task = tm.createTask({ skill: "smart-routing", messages: [] }, "owner-a"); + assert.throws(() => tm.cancelTask(task.id, "owner-b"), /not found/); + assert.equal(tm.getTask(task.id, "owner-a")?.state, "submitted", "task untouched"); + assert.equal(tm.cancelTask(task.id, "owner-a").state, "cancelled", "the owner can cancel"); + }); + + it("owner-scoped listTasks hides other principals' owned tasks", () => { + const tm = makeManager(); + tm.createTask({ skill: "s1", messages: [] }, "owner-a"); + const mine = tm.createTask({ skill: "s1", messages: [] }, "owner-b"); + const listed = tm.listTasks(undefined, "owner-b"); + assert.deepEqual( + listed.map((t) => t.id), + [mine.id] + ); + // No owner scope (management/dashboard path) still sees everything. + assert.equal(tm.listTasks(undefined).length, 2); + }); + + it("ownerless tasks stay visible to everyone (keyless local-first posture)", () => { + const tm = makeManager(); + const task = tm.createTask({ skill: "smart-routing", messages: [] }); + assert.equal(tm.getTask(task.id, "anyone")?.id, task.id); + assert.equal(tm.getTask(task.id)?.id, task.id); + assert.equal(tm.cancelTask(task.id, "anyone").state, "cancelled"); + }); +}); + +describe("REST /api/a2a/tasks/[id] — authentication (GHSA-jcm5)", () => { + it("rejects an unkeyed call when REQUIRE_API_KEY=true (was: no auth at all)", async () => { + process.env.REQUIRE_API_KEY = "true"; + delete process.env.OMNIROUTE_API_KEY; + const res = await restGet.GET(new Request("http://localhost/api/a2a/tasks/abc") as never, { + params: Promise.resolve({ id: "abc" }), + }); + assert.equal(res.status, 401); + }); + + it("serves a keyed call under REQUIRE_API_KEY=true", async () => { + process.env.REQUIRE_API_KEY = "true"; + const key = await apiKeysDb.createApiKey("a2a-rest-client", "machine-rest", []); + const res = await restGet.GET( + new Request("http://localhost/api/a2a/tasks/definitely-missing", { + headers: { authorization: `Bearer ${key.key}` }, + }) as never, + { params: Promise.resolve({ id: "definitely-missing" }) } + ); + // Authenticated — the 404 now comes from the task lookup, not the auth gate. + assert.equal(res.status, 404); + }); + + it("keyed caller gets 404 for another principal's task (route-level IDOR, GHSA-jcm5)", async () => { + process.env.REQUIRE_API_KEY = "true"; + const tm = getTaskManager(); + // A task owned by a DIFFERENT principal than the caller's key hash. + const foreign = tm.createTask({ skill: "smart-routing", messages: [] }, "some-other-owner"); + const key = await apiKeysDb.createApiKey("a2a-rest-idor", "machine-idor", []); + const req = new Request(`http://localhost/api/a2a/tasks/${foreign.id}`, { + headers: { authorization: `Bearer ${key.key}` }, + }); + const res = await restGet.GET(req as never, { params: Promise.resolve({ id: foreign.id }) }); + assert.equal(res.status, 404, "another principal's task is invisible"); + + // And the same task IS visible to its owner (owner hash derived from the key). + const owned = tm.createTask( + { skill: "smart-routing", messages: [] }, + resolveA2AOwner(req as never) + ); + const res2 = await restGet.GET( + new Request(`http://localhost/api/a2a/tasks/${owned.id}`, { + headers: { authorization: `Bearer ${key.key}` }, + }) as never, + { params: Promise.resolve({ id: owned.id }) } + ); + assert.equal(res2.status, 200, "the owner reads its own task"); + }); +}); diff --git a/tests/unit/a2a-tasks-auth.test.ts b/tests/unit/a2a-tasks-auth.test.ts index 5569905d66..53d3a975c3 100644 --- a/tests/unit/a2a-tasks-auth.test.ts +++ b/tests/unit/a2a-tasks-auth.test.ts @@ -8,7 +8,9 @@ const __filename = fileURLToPath(import.meta.url); const __dirname = path.dirname(__filename); const TASKS_ROUTE = path.resolve(__dirname, "../../src/app/api/a2a/tasks/route.ts"); -const A2A_ROUTE = path.resolve(__dirname, "../../src/app/a2a/route.ts"); +// GHSA-jcm5-6wpp-wjj8: the constant-time token comparison moved out of +// src/app/a2a/route.ts into the shared helper both surfaces now use. +const A2A_AUTH_HELPER = path.resolve(__dirname, "../../src/lib/a2a/authenticate.ts"); const source = fs.readFileSync(TASKS_ROUTE, "utf-8"); @@ -21,11 +23,11 @@ function hasImport(src: string, name: string, from: string): boolean { return pattern.test(src); } -test("tasks route uses the same constant-time contract as src/app/a2a/route.ts", () => { - const a2aSource = fs.readFileSync(A2A_ROUTE, "utf-8"); +test("tasks route uses the same constant-time contract as the shared A2A auth helper", () => { + const a2aSource = fs.readFileSync(A2A_AUTH_HELPER, "utf-8"); assert.ok( - hasImport(a2aSource, "timingSafeEqual", "node:crypto"), - "reference route imports timingSafeEqual" + hasImport(a2aSource, "timingSafeEqual", "crypto"), + "shared auth helper imports timingSafeEqual" ); assert.ok( diff --git a/tests/unit/authz/routeGuard.test.ts b/tests/unit/authz/routeGuard.test.ts index 163f5bce41..cae8ef4a8c 100644 --- a/tests/unit/authz/routeGuard.test.ts +++ b/tests/unit/authz/routeGuard.test.ts @@ -22,6 +22,22 @@ test("isLocalOnlyPath: /api/cli-tools/runtime/ is local-only", () => { assert.equal(isLocalOnlyPath("/api/cli-tools/runtime/claude"), true); }); +test("isLocalOnlyPath: MITM management routes are local-only (GHSA-x7vm-hp44-9p79)", () => { + // The "Enable MITM" flow installs a system-wide trusted root CA and writes + // /etc/hosts DNS overrides (src/mitm/*) — host-level TLS interception. Both + // routes were MANAGEMENT-classified only, so requireLogin=false left them + // remotely reachable. They belong to the same loopback tier as + // /api/tools/agent-bridge/ (also MITM + DNS). + assert.equal(isLocalOnlyPath("/api/settings/mitm"), true); + assert.equal(isLocalOnlyPath("/api/cli-tools/antigravity-mitm"), true); + assert.equal(isLocalOnlyPath("/api/cli-tools/antigravity-mitm/alias"), true); +}); + +test("isLocalOnlyBypassableByManageScope: MITM routes are NOT bypassable (GHSA-x7vm-hp44-9p79)", () => { + assert.equal(isLocalOnlyBypassableByManageScope("/api/settings/mitm"), false); + assert.equal(isLocalOnlyBypassableByManageScope("/api/cli-tools/antigravity-mitm"), false); +}); + test("isLocalOnlyPath: regular management routes are not local-only", () => { assert.equal(isLocalOnlyPath("/api/settings"), false); assert.equal(isLocalOnlyPath("/api/providers"), false); @@ -89,6 +105,19 @@ test("isAlwaysProtectedPath: /api/db-backups is always protected (GHSA-mghq-58h3 assert.equal(isAlwaysProtectedPath("/api/db-backups/import"), true); }); +test("isAlwaysProtectedPath: legacy settings export/import-json are always protected (GHSA-v7g9-7f55-5g46)", () => { + // The mghq fix covered /api/db-backups but left the legacy sibling routes out: + // export-json dumps every credential and import-json irreversibly replaces + // settings/connections. Both handlers only check isAuthRequired(), which + // returns false under requireLogin=false — so they must sit in Tier 2 like + // /api/settings/database and /api/db-backups. + assert.equal(isAlwaysProtectedPath("/api/settings/export-json"), true); + assert.equal(isAlwaysProtectedPath("/api/settings/import-json"), true); + // The matcher is a plain startsWith (fail-closed: covers more, never less), + // so a hypothetical export-json2 sibling would also be protected — fine. + assert.equal(isAlwaysProtectedPath("/api/settings/proxy"), false); +}); + test("isAlwaysProtectedPath: ordinary settings routes are not always protected", () => { assert.equal(isAlwaysProtectedPath("/api/settings"), false); assert.equal(isAlwaysProtectedPath("/api/settings/proxy"), false); diff --git a/tests/unit/authz/spawn-capable-prefixes-client-safe.test.ts b/tests/unit/authz/spawn-capable-prefixes-client-safe.test.ts index 788777596d..8a2c6b9e17 100644 --- a/tests/unit/authz/spawn-capable-prefixes-client-safe.test.ts +++ b/tests/unit/authz/spawn-capable-prefixes-client-safe.test.ts @@ -82,11 +82,13 @@ test("SPAWN_CAPABLE_PREFIXES is defined in the server-free constants leaf with t "/api/headroom/stop", "/api/vnc-session", "/api/modality-bridge/video/", + "/api/settings/mitm", + "/api/cli-tools/antigravity-mitm", ]) { assert.ok( SPAWN_CAPABLE_PREFIXES.includes(prefix), `SPAWN_CAPABLE_PREFIXES lost the spawn-capable prefix "${prefix}" during extraction` ); } - assert.equal(SPAWN_CAPABLE_PREFIXES.length, 12); + assert.equal(SPAWN_CAPABLE_PREFIXES.length, 14); }); diff --git a/tests/unit/cli-serve-hostname.test.ts b/tests/unit/cli-serve-hostname.test.ts index 377e7eed38..9b801e2baa 100644 --- a/tests/unit/cli-serve-hostname.test.ts +++ b/tests/unit/cli-serve-hostname.test.ts @@ -1,6 +1,6 @@ import test from "node:test"; import assert from "node:assert/strict"; -import { resolveServerHost } from "../../bin/cli/utils/serverHost.mjs"; +import { resolveServerHost, resolveExposureWarning } from "../../bin/cli/utils/serverHost.mjs"; test("serve hostname: Linux honors OMNIROUTE_SERVER_HOST when HOSTNAME is set", () => { assert.equal( @@ -55,3 +55,26 @@ test("serve hostname: Windows preserves an explicit legacy HOSTNAME", () => { test("serve hostname: Windows ignores an auto-set HOSTNAME matching the machine", () => { assert.equal(resolveServerHost({ HOSTNAME: "windows-pc" }, "win32", "windows-pc"), "0.0.0.0"); }); + +test("exposure warning: fires when bound to all interfaces with no API-key requirement (GHSA-wmgv-ph3p-rv57)", () => { + const warning = resolveExposureWarning({}, "0.0.0.0"); + assert.ok(warning, "a warning must be returned for the shipped default posture"); + assert.match(warning, /REQUIRE_API_KEY/); + assert.match(warning, /OMNIROUTE_SERVER_HOST/); +}); + +test("exposure warning: silent when REQUIRE_API_KEY is enabled", () => { + assert.equal(resolveExposureWarning({ REQUIRE_API_KEY: "true" }, "0.0.0.0"), null); + assert.equal(resolveExposureWarning({ REQUIRE_API_KEY: "1" }, "0.0.0.0"), null); +}); + +test("exposure warning: silent on loopback binds", () => { + assert.equal(resolveExposureWarning({}, "127.0.0.1"), null); + assert.equal(resolveExposureWarning({}, "localhost"), null); + assert.equal(resolveExposureWarning({}, "::1"), null); +}); + +test("exposure warning: fires for a LAN bind too (any non-loopback interface)", () => { + assert.ok(resolveExposureWarning({}, "192.168.0.17")); + assert.ok(resolveExposureWarning({}, "::")); +}); diff --git a/tests/unit/search-baseurl-ssrf-guard.test.ts b/tests/unit/search-baseurl-ssrf-guard.test.ts new file mode 100644 index 0000000000..f42335edd3 --- /dev/null +++ b/tests/unit/search-baseurl-ssrf-guard.test.ts @@ -0,0 +1,81 @@ +/** + * SSRF guard coverage for /v1/search's shared base-url resolution (GHSA-j7j4-g9qc-q69c). + * + * `provider_options.baseUrl` (and legacy `providerSpecificData.baseUrl`) is + * client-controlled and flowed verbatim through `resolveSearchBaseUrl()` into + * every search builder's server-side fetch target (searxng, ollama, …), with + * no SSRF validation — while the sink (`searchProxy.ts`) is a plain `fetch()`. + * The Firecrawl sibling was fixed in #10738; this shared resolver was missed. + * + * Guard mode is `block-metadata` (NOT public-only): the catalog's primary + * searxng use case is a self-hosted instance on loopback/LAN, so private + * hosts must keep working, while cloud-metadata endpoints (IMDS credential + * theft — the worst pivot) are rejected. + * + * Run with: + * node --import tsx/esm --test tests/unit/search-baseurl-ssrf-guard.test.ts + */ + +import { describe, it } from "node:test"; +import assert from "node:assert/strict"; + +import { resolveSearchBaseUrl } from "../../open-sse/handlers/search.ts"; +import type { SearchProviderConfig } from "../../open-sse/config/searchRegistry.ts"; + +const config: SearchProviderConfig = { + id: "searxng-search", + name: "SearXNG", + baseUrl: "http://127.0.0.1:8888", + method: "GET", + authType: "none", + costPerQuery: 0, +} as SearchProviderConfig; + +const base = { + query: "test", + searchType: "web", + maxResults: 5, +}; + +const METADATA_URLS = [ + "http://169.254.169.254/latest/meta-data/iam/security-credentials/", + "http://169.254.169.254/latest/meta-data/?x=/search", // reporter's suffix-bypass shape + "http://metadata.google.internal/computeMetadata/v1/", +]; + +describe("resolveSearchBaseUrl — SSRF guard on client-controlled baseUrl (GHSA-j7j4)", () => { + for (const malicious of METADATA_URLS) { + it(`rejects providerOptions.baseUrl pointing at cloud metadata (${malicious})`, () => { + assert.throws(() => { + resolveSearchBaseUrl(config, { ...base, providerOptions: { baseUrl: malicious } }); + }); + }); + + it(`rejects providerSpecificData.baseUrl pointing at cloud metadata (${malicious})`, () => { + assert.throws(() => { + resolveSearchBaseUrl(config, { ...base, providerSpecificData: { baseUrl: malicious } }); + }); + }); + } + + it("still allows a self-hosted loopback/LAN override (block-metadata, not public-only)", () => { + assert.equal( + resolveSearchBaseUrl(config, { + ...base, + providerOptions: { baseUrl: "http://127.0.0.1:9999" }, + }), + "http://127.0.0.1:9999" + ); + assert.equal( + resolveSearchBaseUrl(config, { + ...base, + providerOptions: { baseUrl: "http://10.0.0.5:8080" }, + }), + "http://10.0.0.5:8080" + ); + }); + + it("leaves the catalog baseUrl untouched when no override is supplied", () => { + assert.equal(resolveSearchBaseUrl(config, base), "http://127.0.0.1:8888"); + }); +});