From eb4fd74b1379a7466a9813511b37f3a04518c29b Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza Date: Fri, 21 Aug 2026 20:28:16 -0300 Subject: [PATCH] =?UTF-8?q?fix(security):=20close=20remaining=20v3.8.50=20?= =?UTF-8?q?advisories=20(batch=202=20=E2=80=94=2011=20findings)=20(#11040)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ⭐5 — Fecha 11 achados restantes das advisories de segurança do v3.8.50 (batch 2), TDD. UNSTABLE é o base-red #9985 já rastreado. --- open-sse/executors/base.ts | 33 ++++++++ open-sse/executors/glm.ts | 1 + open-sse/executors/nlpcloud.ts | 1 + open-sse/mcp-server/tools/memoryTools.ts | 22 ++++-- src/app/a2a/route.ts | 29 +++++-- src/app/api/monitoring/health/route.ts | 22 +++++- .../api/oauth/[provider]/[action]/route.ts | 11 +++ src/app/api/oauth/cliproxy-import/route.ts | 8 +- src/app/api/oauth/codex/import-token/route.ts | 11 +-- src/app/api/oauth/codex/import/route.ts | 10 +-- src/app/api/oauth/cursor/auto-import/route.ts | 10 +-- src/app/api/oauth/cursor/import/route.ts | 8 +- src/app/api/oauth/kiro/auto-import/route.ts | 10 +-- src/app/api/oauth/kiro/import/route.ts | 8 +- .../api/oauth/raycast/auto-import/route.ts | 8 +- src/app/api/oauth/raycast/import/route.ts | 8 +- src/app/api/oauth/trae/import/route.ts | 8 +- src/app/api/settings/obsidian/webdav/route.ts | 12 ++- src/lib/services/ServiceSupervisor.ts | 9 ++- src/lib/services/portProbe.ts | 38 +++++++++- src/server/authz/routeGuard.ts | 2 + src/server/cors/origins.ts | 30 +++++++- src/shared/constants/publicApiRoutes.ts | 19 +++++ src/shared/utils/apiKeyPolicy.ts | 30 +++++++- tests/unit/a2a-route-require-api-key.test.ts | 69 +++++++++++++++++ tests/unit/api-key-policy.test.ts | 34 +++++++++ .../authz/oauth-autoimport-local-only.test.ts | 35 +++++++++ tests/unit/base-executor-ssrf-guard.test.ts | 38 ++++++++++ tests/unit/cors/origins.test.ts | 40 ++++++++-- tests/unit/mcp-memory-tools-strategy.test.ts | 34 +++++++++ .../monitoring-health-public-view.test.ts | 48 ++++++++++++ tests/unit/ninerouter-embed-port-6205.test.ts | 19 +++-- .../oauth-device-code-region-ssrf.test.ts | 54 +++++++++++++ tests/unit/oauth-import-manage-scope.test.ts | 75 +++++++++++++++++++ tests/unit/obsidian-webdav-route.test.ts | 41 +++++++++- 35 files changed, 749 insertions(+), 86 deletions(-) create mode 100644 tests/unit/a2a-route-require-api-key.test.ts create mode 100644 tests/unit/authz/oauth-autoimport-local-only.test.ts create mode 100644 tests/unit/base-executor-ssrf-guard.test.ts create mode 100644 tests/unit/monitoring-health-public-view.test.ts create mode 100644 tests/unit/oauth-device-code-region-ssrf.test.ts create mode 100644 tests/unit/oauth-import-manage-scope.test.ts diff --git a/open-sse/executors/base.ts b/open-sse/executors/base.ts index 877431b68e..66e10b67e9 100644 --- a/open-sse/executors/base.ts +++ b/open-sse/executors/base.ts @@ -104,6 +104,12 @@ import { import { applyPeerTraceHeader } from "@/shared/resilience/peerRouting"; import { applyClineProtocolHeaders } from "@/shared/utils/clineAuth"; import { isProbeContext } from "@/shared/utils/probeOrigin"; +import { + parseAndValidatePublicUrl, + parseAndValidateNonMetadataUrl, +} from "@/shared/network/outboundUrlGuard"; +import { getProviderValidationGuard } from "@/shared/network/outboundUrlGuardPolicy"; +import { isLocalProvider, isSelfHostedChatProvider } from "@/shared/constants/providers"; // Header helpers extracted to a pure leaf; re-exported for external importers // (executors + tests) that import them from "./base.ts". export { @@ -397,6 +403,29 @@ export class BaseExecutor { return fallback || this.config.baseUrl || ""; } + /** + * SSRF guard for the runtime dispatch path (GHSA-4f49-hj64-448x). A persisted, + * caller-supplied `providerSpecificData.baseUrl` reaches the fetch() calls + * below, so a `manage`-scope actor (or, on a keyless install, an anonymous + * one) could point a provider at loopback / internal / cloud-metadata hosts + * and exfiltrate the stored upstream key. Mirror the provider VALIDATION + * guard so runtime dispatch makes the same decision the validation layer + * already makes: local / self-hosted providers are exempt (they legitimately + * use private URLs, and the OMNIROUTE_ALLOW_PRIVATE_PROVIDER_URLS opt-in still + * applies through the guard), and for everything else `public-only` mode + * blocks private + metadata while the default `block-metadata` mode blocks the + * cloud-metadata IMDS pivot. Throws on a blocked URL. + */ + protected assertOutboundUrlAllowed(url: string): void { + if (!url) return; + if (isLocalProvider(this.provider) || isSelfHostedChatProvider(this.provider)) return; + if (getProviderValidationGuard() === "public-only") { + parseAndValidatePublicUrl(url); + return; + } + parseAndValidateNonMetadataUrl(url); + } + /** * Alternate protocol selected on this connection, if the provider declares one * that matches. Centralizes the registry lookup so every call-site resolves the @@ -615,6 +644,7 @@ export class BaseExecutor { async countTokens({ model, body, credentials, signal, log }: CountTokensInput) { const url = this.buildCountTokensUrl(model, credentials); if (!url) return null; + this.assertOutboundUrlAllowed(url); // GHSA-4f49 const headers = this.buildHeaders(credentials, false); const requestBody = @@ -869,6 +899,9 @@ export class BaseExecutor { // Timeout only covers response start; stream stalls are handled downstream. const fetchStartTimeoutMs = this.getTimeoutMs(); const fetchWithStartTimeout = async (requestUrl: string, requestOptions: RequestInit) => { + // GHSA-4f49: guard here (not only next to the first buildUrl) so retries + // and fallback URLs are validated too, before any bytes leave the host. + this.assertOutboundUrlAllowed(requestUrl); const timeoutController = fetchStartTimeoutMs > 0 ? new AbortController() : null; let timeoutId: ReturnType | null = null; if (timeoutController) { diff --git a/open-sse/executors/glm.ts b/open-sse/executors/glm.ts index 6318aaab2e..a222571022 100644 --- a/open-sse/executors/glm.ts +++ b/open-sse/executors/glm.ts @@ -430,6 +430,7 @@ export class GlmExecutor extends DefaultExecutor { let response: Response; try { + this.assertOutboundUrlAllowed(url); // GHSA-4f49: glm has its own fetch path response = await fetch(url, { method: "POST", headers, diff --git a/open-sse/executors/nlpcloud.ts b/open-sse/executors/nlpcloud.ts index d413b5a683..e212a38efe 100644 --- a/open-sse/executors/nlpcloud.ts +++ b/open-sse/executors/nlpcloud.ts @@ -471,6 +471,7 @@ export class NlpCloudExecutor extends BaseExecutor { } try { + this.assertOutboundUrlAllowed(url); // GHSA-4f49: nlpcloud has its own fetch path const response = await fetch(url, { method: "POST", headers, diff --git a/open-sse/mcp-server/tools/memoryTools.ts b/open-sse/mcp-server/tools/memoryTools.ts index 16c835fd8e..12a8c99c0d 100644 --- a/open-sse/mcp-server/tools/memoryTools.ts +++ b/open-sse/mcp-server/tools/memoryTools.ts @@ -10,16 +10,24 @@ import { import { resolveMcpCallerApiKeyId } from "../mcpCallerIdentity.ts"; /** - * Resolve the memory owner id for an MCP tool call: - * explicit arg wins, otherwise fall back to the authenticated caller's - * principal id (HTTP auth headers on SSE/Streamable HTTP transports, - * OMNIROUTE_API_KEY env var on stdio). Keeps MCP-stored memories under - * the same owner id that chat-context memory uses, so retrieval in the - * chat pipeline finds entries written via MCP. + * Resolve the memory owner id for an MCP tool call. + * + * The authenticated caller's principal ALWAYS wins over a caller-supplied + * `apiKeyId` — otherwise any MCP caller could read, write, or delete another + * principal's memories by putting a different id in the tool arguments + * (GHSA-cpv3-xr7r-xf8q, IDOR). The caller is resolved from the per-request HTTP + * auth headers on SSE / Streamable HTTP transports, or from OMNIROUTE_API_KEY on + * stdio. The explicit argument is only honored as a fallback when no caller can + * be resolved (a bare local stdio process with no configured key — already + * trusted), preserving the local-tooling flow. Keeps MCP-stored memories under + * the same owner id that chat-context memory uses, so retrieval in the chat + * pipeline finds entries written via MCP. */ async function resolveMemoryOwnerId(explicit?: string): Promise { + const caller = await resolveMcpCallerApiKeyId().catch(() => undefined); + if (caller) return caller; if (explicit && explicit.trim() !== "") return explicit.trim(); - return (await resolveMcpCallerApiKeyId().catch(() => undefined)) || "mcp"; + return "mcp"; } export const MemorySearchSchema = z.object({ diff --git a/src/app/a2a/route.ts b/src/app/a2a/route.ts index e9e90a99ab..af7d93a2e5 100644 --- a/src/app/a2a/route.ts +++ b/src/app/a2a/route.ts @@ -17,6 +17,8 @@ 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"; // ============ A2A v1.0 ↔ v0.3 compatibility layer ============ // A2A 1.0 renamed the JSON-RPC methods (message/send → SendMessage, @@ -136,14 +138,25 @@ function tokensMatch(provided: string, expected: string): boolean { return timingSafeEqual(a, b); } -function authenticate(req: NextRequest): boolean { - // If no API key is configured, allow all requests - const configuredKey = process.env.OMNIROUTE_API_KEY; - if (!configuredKey) return true; +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 authHeader = req.headers.get("authorization") || ""; - const token = authHeader.replace(/^Bearer\s+/i, ""); - return tokensMatch(token, configuredKey); + 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; } // ============ JSON-RPC Helpers ============ @@ -179,7 +192,7 @@ async function rejectIfA2ADisabled(id: string | number | null) { export async function POST(req: NextRequest) { // Auth check - if (!authenticate(req)) { + if (!(await authenticate(req))) { return jsonRpcError(null, -32600, "Unauthorized: missing or invalid API key"); } diff --git a/src/app/api/monitoring/health/route.ts b/src/app/api/monitoring/health/route.ts index 144d0a3ce0..f3dceac558 100644 --- a/src/app/api/monitoring/health/route.ts +++ b/src/app/api/monitoring/health/route.ts @@ -5,6 +5,7 @@ import { readRunningBuildSha } from "@/lib/monitoring/buildSha"; import { APP_CONFIG } from "@/shared/constants/config"; import { AI_PROVIDERS } from "@/shared/constants/providers"; import { isAuthenticated } from "@/shared/utils/apiAuth"; +import { requireManagementAuth } from "@/lib/api/requireManagementAuth"; /** * GET /api/monitoring/health — System health overview @@ -20,10 +21,25 @@ import { isAuthenticated } from "@/shared/utils/apiAuth"; let healthPayloadCache: { payload: unknown; expiresAt: number } | null = null; const HEALTH_PAYLOAD_TTL_MS = 1000; -export async function GET() { +// GHSA-mvf8-qc78-5mxm: the full health payload fingerprints the host (version, +// node version, pid, memory, provider config). An anonymous caller — the common +// case on a keyless install, and what a liveness/load-balancer probe needs — gets +// only the liveness verdict; the detail is reserved for a management principal. +function publicHealthView(payload: unknown): Record { + const p = (payload ?? {}) as Record; + return { + status: p.status ?? "unknown", + ...(p.setupComplete !== undefined ? { setupComplete: p.setupComplete } : {}), + }; +} + +export async function GET(request: Request) { + const fullView = (await requireManagementAuth(request, { alwaysRequireAuth: true })) === null; const cachedNow = Date.now(); if (healthPayloadCache && cachedNow <= healthPayloadCache.expiresAt) { - return NextResponse.json(healthPayloadCache.payload); + return NextResponse.json( + fullView ? healthPayloadCache.payload : publicHealthView(healthPayloadCache.payload) + ); } const readHealthValue = (label: string, reader: () => T, fallback: T): T => { @@ -187,7 +203,7 @@ export async function GET() { }); healthPayloadCache = { payload, expiresAt: Date.now() + HEALTH_PAYLOAD_TTL_MS }; - return NextResponse.json(payload); + return NextResponse.json(fullView ? payload : publicHealthView(payload)); } catch (error) { console.error("[API] GET /api/monitoring/health error:", error); return NextResponse.json({ diff --git a/src/app/api/oauth/[provider]/[action]/route.ts b/src/app/api/oauth/[provider]/[action]/route.ts index 7b71a07758..b52bae4201 100755 --- a/src/app/api/oauth/[provider]/[action]/route.ts +++ b/src/app/api/oauth/[provider]/[action]/route.ts @@ -24,6 +24,7 @@ import { } from "@/models"; import { getConsistentMachineId } from "@/shared/utils/machineId"; import { isValidGheUrl } from "@/shared/validation/providerSpecificData"; +import { AWS_REGION_PATTERN } from "@/lib/oauth/constants/oauth"; import { syncToCloud } from "@/lib/cloudSync"; import { startLocalServer } from "@/lib/oauth/utils/server"; import { runWithProxyContextOrDirect } from "@omniroute/open-sse/utils/proxyFetch.ts"; @@ -221,6 +222,16 @@ export async function GET( (requestDeviceCode as any)(provider, null, providerOverrideConfig) ); } else if ((provider === "kiro" || provider === "amazon-q") && startUrl) { + // GHSA-7x63: `region` is interpolated into the AWS OIDC endpoint URLs + // below, which requestDeviceCode() then fetches. Validate it against the + // canonical AWS region shape before it can steer the outbound host to an + // attacker-chosen target (userinfo/fragment tricks → SSRF / metadata). + if (!AWS_REGION_PATTERN.test(region)) { + return NextResponse.json( + { error: "region must be a valid AWS region (e.g. us-east-1)" }, + { status: 400 } + ); + } const providerOverrideConfig = { ...providerData.config, startUrl, diff --git a/src/app/api/oauth/cliproxy-import/route.ts b/src/app/api/oauth/cliproxy-import/route.ts index 7c60ba9775..ca579983b8 100644 --- a/src/app/api/oauth/cliproxy-import/route.ts +++ b/src/app/api/oauth/cliproxy-import/route.ts @@ -3,7 +3,7 @@ import path from "path"; import { NextResponse } from "next/server"; import { createProviderConnection } from "@/models"; -import { isAuthRequired, isAuthenticated } from "@/shared/utils/apiAuth"; +import { requireManagementAuth } from "@/lib/api/requireManagementAuth"; import { sanitizeErrorMessage } from "@omniroute/open-sse/utils/error"; import { scanCliProxyAuthDir, @@ -23,9 +23,9 @@ function cliProxyConfigDir(): string { } async function requireImportAuth(request: Request) { - if (!(await isAuthRequired(request))) return null; - if (await isAuthenticated(request)) return null; - return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); + // GHSA-mg76: importing a provider connection is a state-mutating admin action; + // require management scope (or a dashboard session), not any valid client key. + return requireManagementAuth(request, { invalidApiKeyStatus: 401 }); } export async function GET(request: Request) { diff --git a/src/app/api/oauth/codex/import-token/route.ts b/src/app/api/oauth/codex/import-token/route.ts index 4601b2ae2d..a5b22c4e37 100644 --- a/src/app/api/oauth/codex/import-token/route.ts +++ b/src/app/api/oauth/codex/import-token/route.ts @@ -3,7 +3,7 @@ import { z } from "zod"; import { extractCodexAccountInfo } from "@/lib/oauth/services/codexImport"; import { parseCodexSessionJson } from "@/lib/oauth/utils/codexSessionImport"; import { createProviderConnection } from "@/models"; -import { isAuthRequired, isAuthenticated } from "@/shared/utils/apiAuth"; +import { requireManagementAuth } from "@/lib/api/requireManagementAuth"; import { buildErrorBody, sanitizeErrorMessage } from "@omniroute/open-sse/utils/error.ts"; /** @@ -93,10 +93,11 @@ async function parseRequestBody( return { ok: true, resolved: resolved.resolved }; } -async function requireAuth(request: Request): Promise { - if (!(await isAuthRequired(request))) return null; - if (await isAuthenticated(request)) return null; - return NextResponse.json(buildErrorBody(401, "Unauthorized"), { status: 401 }); +async function requireAuth(request: Request): Promise { + // GHSA-mg76: importing a provider connection is a state-mutating admin action. + // Require management scope (or a dashboard session) rather than accepting any + // valid client key, which the PUBLIC /api/oauth/ classification otherwise allows. + return requireManagementAuth(request, { invalidApiKeyStatus: 401 }); } export async function POST(request: Request) { diff --git a/src/app/api/oauth/codex/import/route.ts b/src/app/api/oauth/codex/import/route.ts index a7302a3a6d..6ad9007261 100644 --- a/src/app/api/oauth/codex/import/route.ts +++ b/src/app/api/oauth/codex/import/route.ts @@ -2,7 +2,7 @@ import { NextResponse } from "next/server"; import { z } from "zod"; import { normalizeCodexImportRecord, flattenCodexImportPayload } from "@/lib/oauth/services/codexImport"; import { createProviderConnection } from "@/models"; -import { isAuthRequired, isAuthenticated } from "@/shared/utils/apiAuth"; +import { requireManagementAuth } from "@/lib/api/requireManagementAuth"; import { sanitizeErrorMessage } from "@omniroute/open-sse/utils/error.ts"; import { refreshCodexToken, isUnrecoverableRefreshError } from "@omniroute/open-sse/services/tokenRefresh.ts"; @@ -82,10 +82,10 @@ const bodySchema = z.object({ }), }); -async function requireAuth(request: Request): Promise { - if (!(await isAuthRequired(request))) return null; - if (await isAuthenticated(request)) return null; - return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); +async function requireAuth(request: Request): Promise { + // GHSA-mg76: importing a provider connection is a state-mutating admin action; + // require management scope (or a dashboard session), not any valid client key. + return requireManagementAuth(request, { invalidApiKeyStatus: 401 }); } export async function POST(request: Request) { diff --git a/src/app/api/oauth/cursor/auto-import/route.ts b/src/app/api/oauth/cursor/auto-import/route.ts index c7ba57c419..7a1fcdda3f 100755 --- a/src/app/api/oauth/cursor/auto-import/route.ts +++ b/src/app/api/oauth/cursor/auto-import/route.ts @@ -1,5 +1,5 @@ import { NextResponse } from "next/server"; -import { isAuthRequired, isAuthenticated } from "@/shared/utils/apiAuth"; +import { requireManagementAuth } from "@/lib/api/requireManagementAuth"; import { tryAgentAuth, tryIdeAuth } from "@/lib/cursor/tokenExtractor"; /** @@ -11,11 +11,9 @@ import { tryAgentAuth, tryIdeAuth } from "@/lib/cursor/tokenExtractor"; * 🔒 Auth-guarded: requires JWT cookie or Bearer API key (finding #258-4). */ export async function GET(request: Request) { - if (await isAuthRequired(request)) { - if (!(await isAuthenticated(request))) { - return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); - } - } + // GHSA-mg76 / GHSA-gxv4: reading/importing host credentials is a management action. + const authError = await requireManagementAuth(request, { invalidApiKeyStatus: 401 }); + if (authError) return authError; try { // Try Cursor IDE first (has both accessToken and machineId) diff --git a/src/app/api/oauth/cursor/import/route.ts b/src/app/api/oauth/cursor/import/route.ts index 845946cae9..89b8c26745 100755 --- a/src/app/api/oauth/cursor/import/route.ts +++ b/src/app/api/oauth/cursor/import/route.ts @@ -6,15 +6,15 @@ import { isCloudEnabled } from "@/models"; import { syncToCloud } from "@/lib/cloudSync"; import { cursorImportSchema } from "@/shared/validation/schemas"; import { isValidationFailure, validateBody } from "@/shared/validation/helpers"; -import { isAuthRequired, isAuthenticated } from "@/shared/utils/apiAuth"; +import { requireManagementAuth } from "@/lib/api/requireManagementAuth"; import { getConsistentMachineId } from "@/shared/utils/machineId"; import { runWithProxyContext } from "@omniroute/open-sse/utils/proxyFetch.ts"; import { resolveProxyForProvider } from "@/models"; async function requireOAuthImportAuth(request: Request) { - if (!(await isAuthRequired(request))) return null; - if (await isAuthenticated(request)) return null; - return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); + // GHSA-mg76: importing a provider connection is a state-mutating admin action; + // require management scope (or a dashboard session), not any valid client key. + return requireManagementAuth(request, { invalidApiKeyStatus: 401 }); } /** diff --git a/src/app/api/oauth/kiro/auto-import/route.ts b/src/app/api/oauth/kiro/auto-import/route.ts index 40b927b4c9..ca61b177dd 100755 --- a/src/app/api/oauth/kiro/auto-import/route.ts +++ b/src/app/api/oauth/kiro/auto-import/route.ts @@ -1,7 +1,7 @@ import { NextResponse } from "next/server"; import { homedir } from "os"; import { join } from "path"; -import { isAuthRequired, isAuthenticated } from "@/shared/utils/apiAuth"; +import { requireManagementAuth } from "@/lib/api/requireManagementAuth"; import { createProviderConnection, getProviderConnections, @@ -31,11 +31,9 @@ import { * 🔒 Auth-guarded: requires JWT cookie or Bearer API key. */ export async function GET(request: Request) { - if (await isAuthRequired(request)) { - if (!(await isAuthenticated(request))) { - return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); - } - } + // GHSA-mg76 / GHSA-gxv4: reading/importing host credentials is a management action. + const authError = await requireManagementAuth(request, { invalidApiKeyStatus: 401 }); + if (authError) return authError; const { searchParams } = new URL(request.url); const targetProvider = searchParams.get("targetProvider") === "amazon-q" ? "amazon-q" : "kiro"; diff --git a/src/app/api/oauth/kiro/import/route.ts b/src/app/api/oauth/kiro/import/route.ts index d4b89183ab..29aa19d07f 100755 --- a/src/app/api/oauth/kiro/import/route.ts +++ b/src/app/api/oauth/kiro/import/route.ts @@ -11,7 +11,7 @@ import { getConsistentMachineId } from "@/shared/utils/machineId"; import { syncToCloud } from "@/lib/cloudSync"; import { kiroImportSchema } from "@/shared/validation/schemas"; import { isValidationFailure, validateBody } from "@/shared/validation/helpers"; -import { isAuthRequired, isAuthenticated } from "@/shared/utils/apiAuth"; +import { requireManagementAuth } from "@/lib/api/requireManagementAuth"; import { runWithProxyContext } from "@omniroute/open-sse/utils/proxyFetch.ts"; import { sanitizeErrorMessage } from "@omniroute/open-sse/utils/error"; import { findKiroConnectionByIdentity } from "@/lib/oauth/kiroConnectionIdentity"; @@ -38,9 +38,9 @@ export function buildKiroImportError(error: unknown): string { } async function requireOAuthImportAuth(request: Request) { - if (!(await isAuthRequired(request))) return null; - if (await isAuthenticated(request)) return null; - return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); + // GHSA-mg76: importing a provider connection is a state-mutating admin action; + // require management scope (or a dashboard session), not any valid client key. + return requireManagementAuth(request, { invalidApiKeyStatus: 401 }); } async function upsertImportedKiroConnection( diff --git a/src/app/api/oauth/raycast/auto-import/route.ts b/src/app/api/oauth/raycast/auto-import/route.ts index 17e4cf48c5..4dc3ac8001 100644 --- a/src/app/api/oauth/raycast/auto-import/route.ts +++ b/src/app/api/oauth/raycast/auto-import/route.ts @@ -14,14 +14,14 @@ import { extractLocalRaycastCredentials, isRaycastLocalExtractAvailable, } from "@/lib/oauth/services/raycastLocal"; -import { isAuthRequired, isAuthenticated } from "@/shared/utils/apiAuth"; +import { requireManagementAuth } from "@/lib/api/requireManagementAuth"; import { resolveProxyForProvider } from "@/models"; import { runWithProxyContext } from "@omniroute/open-sse/utils/proxyFetch.ts"; async function requireOAuthImportAuth(request: Request) { - if (!(await isAuthRequired(request))) return null; - if (await isAuthenticated(request)) return null; - return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); + // GHSA-mg76: importing a provider connection is a state-mutating admin action; + // require management scope (or a dashboard session), not any valid client key. + return requireManagementAuth(request, { invalidApiKeyStatus: 401 }); } export async function GET(request: Request) { diff --git a/src/app/api/oauth/raycast/import/route.ts b/src/app/api/oauth/raycast/import/route.ts index 0ff02764fa..266dd1aecc 100644 --- a/src/app/api/oauth/raycast/import/route.ts +++ b/src/app/api/oauth/raycast/import/route.ts @@ -11,14 +11,14 @@ import { createProviderConnection } from "@/models"; import { RaycastService } from "@/lib/oauth/services/raycast"; import { raycastImportSchema } from "@/shared/validation/schemas"; import { isValidationFailure, validateBody } from "@/shared/validation/helpers"; -import { isAuthRequired, isAuthenticated } from "@/shared/utils/apiAuth"; +import { requireManagementAuth } from "@/lib/api/requireManagementAuth"; import { resolveProxyForProvider } from "@/models"; import { runWithProxyContext } from "@omniroute/open-sse/utils/proxyFetch.ts"; async function requireOAuthImportAuth(request: Request) { - if (!(await isAuthRequired(request))) return null; - if (await isAuthenticated(request)) return null; - return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); + // GHSA-mg76: importing a provider connection is a state-mutating admin action; + // require management scope (or a dashboard session), not any valid client key. + return requireManagementAuth(request, { invalidApiKeyStatus: 401 }); } export async function POST(request: Request) { diff --git a/src/app/api/oauth/trae/import/route.ts b/src/app/api/oauth/trae/import/route.ts index 9f3fba5bac..c3faeb8e90 100644 --- a/src/app/api/oauth/trae/import/route.ts +++ b/src/app/api/oauth/trae/import/route.ts @@ -2,7 +2,7 @@ import { NextResponse } from "next/server"; import { createProviderConnection } from "@/models"; import { traeImportSchema } from "@/shared/validation/schemas"; import { isValidationFailure, validateBody } from "@/shared/validation/helpers"; -import { isAuthRequired, isAuthenticated } from "@/shared/utils/apiAuth"; +import { requireManagementAuth } from "@/lib/api/requireManagementAuth"; /** * POST /api/oauth/trae/import @@ -22,9 +22,9 @@ import { isAuthRequired, isAuthenticated } from "@/shared/utils/apiAuth"; * region — optional, default "US-East" */ async function requireOAuthImportAuth(request: Request) { - if (!(await isAuthRequired(request))) return null; - if (await isAuthenticated(request)) return null; - return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); + // GHSA-mg76: importing a provider connection is a state-mutating admin action; + // require management scope (or a dashboard session), not any valid client key. + return requireManagementAuth(request, { invalidApiKeyStatus: 401 }); } export async function POST(request: Request) { diff --git a/src/app/api/settings/obsidian/webdav/route.ts b/src/app/api/settings/obsidian/webdav/route.ts index 1465b1dafd..4b098cfdb0 100644 --- a/src/app/api/settings/obsidian/webdav/route.ts +++ b/src/app/api/settings/obsidian/webdav/route.ts @@ -1,6 +1,7 @@ import { NextRequest, NextResponse } from "next/server"; import { z } from "zod"; import { isAuthenticated } from "@/shared/utils/apiAuth"; +import { requireManagementAuth } from "@/lib/api/requireManagementAuth"; import { buildErrorBody } from "@omniroute/open-sse/utils/error"; import { getObsidianSyncStatus, @@ -21,10 +22,19 @@ export async function GET(request: NextRequest) { try { const status = await getObsidianSyncStatus(); + // GHSA-62vw: the WebDAV password is reusable authentication material. Return + // the plaintext only to a genuine management principal (dashboard session or + // manage-scope key), never to an anonymous caller that reached this handler + // through the requireLogin=false open mode. The dashboard's authenticated + // reveal-password view is unaffected; anonymous callers get a set/unset flag. + const hasManagement = + (await requireManagementAuth(request, { alwaysRequireAuth: true })) === null; return NextResponse.json({ webdavEnabled: status.webdavEnabled, webdavUsername: status.webdavEnabled ? status.webdavUsername : null, - webdavPassword: status.webdavEnabled ? status.webdavPassword : null, + webdavPassword: + status.webdavEnabled && hasManagement ? status.webdavPassword : null, + webdavPasswordSet: status.webdavEnabled && Boolean(status.webdavPassword), vaultPath: status.vaultPath, }); } catch (error) { diff --git a/src/lib/services/ServiceSupervisor.ts b/src/lib/services/ServiceSupervisor.ts index 9ec8e44f6a..df1011bb45 100644 --- a/src/lib/services/ServiceSupervisor.ts +++ b/src/lib/services/ServiceSupervisor.ts @@ -7,7 +7,12 @@ import { sanitizeErrorMessage } from "@omniroute/open-sse/utils/error"; import { getServiceRow, updateServiceField, setToolStatus } from "@/lib/db/versionManager"; import { RingBuffer } from "./ringBuffer"; import { HealthChecker } from "./healthCheck"; -import { decidePreSpawn, probeBeforeSpawn, resolvePortPid } from "./portProbe"; +import { + decidePreSpawn, + isAdoptExistingEnabled, + probeBeforeSpawn, + resolvePortPid, +} from "./portProbe"; import type { ServiceConfig, ServiceState, ServiceStatus, LogLine, HealthState } from "./types"; const CRASH_FAST_THRESHOLD_MS = 5_000; @@ -111,7 +116,7 @@ export class ServiceSupervisor extends EventEmitter { // Opt-in per ServiceConfig so the default spawn path is unchanged. if (this.config.probeBeforeSpawn) { const probe = await probeBeforeSpawn(this.config.healthUrl(), this.config.port); - const decision = decidePreSpawn(probe, this.config.port); + const decision = decidePreSpawn(probe, this.config.port, isAdoptExistingEnabled()); if (decision.action === "adopt") { // Something healthy already serves this port. We didn't spawn it, diff --git a/src/lib/services/portProbe.ts b/src/lib/services/portProbe.ts index 81c3d8a846..2a9fd502bd 100644 --- a/src/lib/services/portProbe.ts +++ b/src/lib/services/portProbe.ts @@ -37,11 +37,30 @@ const PID_RESOLVE_TIMEOUT_MS = 2_000; * * Pure — no I/O — so it can be exhaustively unit-tested. */ -export function decidePreSpawn(probe: PreSpawnProbe, port: number): PreSpawnDecision { - // A healthy instance is already serving on the port — adopt it rather than - // spawn a duplicate that would immediately die with EADDRINUSE. +export function decidePreSpawn( + probe: PreSpawnProbe, + port: number, + allowAdopt = false +): PreSpawnDecision { if (probe.healthy) { - return { action: "adopt" }; + // A 2xx on the health path does NOT prove the listener is our service: a + // local process can squat the port, answer 200, and get adopted — receiving + // the injected service API key and script execution inside the dashboard + // origin (GHSA-wg9p-6m2g-4v27). Adopt an already-healthy listener only when + // the operator explicitly opts in; otherwise surface the same actionable + // error we already use for a held-but-unhealthy port instead of silently + // trusting the listener. + if (allowAdopt) { + return { action: "adopt" }; + } + return { + action: "error", + message: + `Port ${port} is already serving a healthy response, but adopting an ` + + `existing listener is disabled by default (a 2xx cannot prove the listener ` + + `is this service). Set OMNIROUTE_ADOPT_EXISTING_SERVICE=1 to allow adoption, ` + + `or stop the process holding the port and start the service again.`, + }; } // Port is held but nothing healthy answers: an orphaned or unrelated process // is squatting on it. Surface a clear, actionable error instead of letting @@ -59,6 +78,17 @@ export function decidePreSpawn(probe: PreSpawnProbe, port: number): PreSpawnDeci return { action: "spawn" }; } +/** + * Whether the operator opted in to adopting an already-healthy listener on a + * service port. Off by default (GHSA-wg9p-6m2g-4v27): a squatter can answer a + * 2xx, so auto-adoption is only safe when the operator knows the listener is + * genuinely their (externally-managed) instance. + */ +export function isAdoptExistingEnabled(env: NodeJS.ProcessEnv = process.env): boolean { + const v = env.OMNIROUTE_ADOPT_EXISTING_SERVICE; + return v === "1" || v === "true"; +} + /** TCP connect check: resolves true when something accepts a connection. */ function isPortInUse(port: number, timeoutMs: number): Promise { return new Promise((resolve) => { diff --git a/src/server/authz/routeGuard.ts b/src/server/authz/routeGuard.ts index d59e68836f..7c61d24545 100644 --- a/src/server/authz/routeGuard.ts +++ b/src/server/authz/routeGuard.ts @@ -56,6 +56,8 @@ export const LOCAL_ONLY_API_PREFIXES: ReadonlyArray = [ "/api/jobs", // JobRegistry control (enable/disable/run-now) + run history - runtime job administration, loopback-only (Hard Rules #15 + #17) "/api/jobs/", // sub-paths: /api/jobs/:id/{runs,enable,disable,run-now} (the bare `/api/jobs` above matches the list route; this matches children) "/api/oauth/cursor/auto-import", // spawns execFile("which", argv-array-of-one-arg "cursor") to verify a local Cursor install before importing creds — RCE-via-tunnel surface (Hard Rules #15 + #17, found by 6A.8 route-guard gate). Specific path only: the rest of /api/oauth/ (browser redirect/callback flows) must stay remote-reachable. Note: this comment intentionally avoids a literal closing square bracket character — check-openapi-security-tiers.mjs's naive regex parser for this array stops at the first one it finds, silently truncating its view of every entry after this one. + "/api/oauth/kiro/auto-import", // reads host-local Kiro credential files (homedir kiro-cli data) — must reach the loopback-only gate, not the PUBLIC /api/oauth/ prefix (GHSA-wgwc-crjm-pmwv, GHSA-gxv4-955v-v6cm). Excluded from PUBLIC in publicApiRoutes.ts. + "/api/oauth/raycast/auto-import", // reads host-local Raycast credential files — same loopback-only rationale as the kiro and cursor auto-import routes above. "/api/skills/collect/", // Skill Collector CLI detection: GET .../detect probes getCliRuntimeStatus() per CLI_TOOL_IDS entry, which spawns a child process to check each tool — RCE-via-tunnel surface (Hard Rules #15 + #17, PR #6294 review). "/api/discovery/", // Discovery tool (opt-in provider scanner): the scan route makes outbound probes to provider endpoints (SSRF-adjacent) and the whole surface is an admin research tool — strict-loopback only, no manage-scope bypass (NOT in LOCAL_ONLY_MANAGE_SCOPE_BYPASS_PREFIXES). See _tasks/features-v3.8.42/gaps/DISCOVERY_TOOL_DESIGN.md. VNC_ROUTE_PREFIX, // #7892: /api/vnc-session/* spawns Docker containers via child_process.spawn (src/lib/vncSession/service.ts) — RCE-via-tunnel surface (Hard Rules #15 + #17), same CVE class (GHSA-fhh6-4qxv-rpqj). diff --git a/src/server/cors/origins.ts b/src/server/cors/origins.ts index dfd0b407c5..a354cccb5e 100644 --- a/src/server/cors/origins.ts +++ b/src/server/cors/origins.ts @@ -144,6 +144,26 @@ export function getCorsStatus(): CorsStatus { * compression middleware only appends it conditionally, so shared caches can't * otherwise reliably tell compressed vs uncompressed variants apart. */ +function requestCarriesTokenOrPreflight(request: Request): boolean { + // Preflight (OPTIONS) never carries the Authorization / x-api-key header, so it + // must be allowed through — the actual request that follows is re-evaluated by + // this same check and only gets the permissive Origin if it presents a token. + if (request.method === "OPTIONS") return true; + if ( + request.headers.get("authorization") || + request.headers.get("x-api-key") || + request.headers.get("x-goog-api-key") + ) { + return true; + } + // A dashboard session cookie is a credential too (#5242 browser/Electron + // clients). auth_token is HttpOnly + SameSite, so a cross-site attacker page + // cannot get it auto-attached — only a truly credential-less request (the + // GHSA-7px7 anonymous case on a keyless install) falls through to fail-closed. + const cookie = request.headers.get("cookie"); + return Boolean(cookie && /(?:^|;\s*)auth_token=/.test(cookie)); +} + export function applyCorsHeaders( response: Response, request: Request, @@ -151,7 +171,15 @@ export function applyCorsHeaders( ): void { const requestOrigin = request.headers.get("origin"); let allowed = resolveAllowedOrigin(requestOrigin); - if (allowed === null && relaxForTokenAuth) { + if (allowed === null && relaxForTokenAuth && requestCarriesTokenOrPreflight(request)) { + // GHSA-7px7-29v2-m97p: the permissive Origin echo is only safe on the + // assumption that these routes are token-authenticated (browsers never + // auto-attach Authorization/x-api-key). On a keyless install that assumption + // breaks — an anonymous cross-origin page would be echoed its own Origin and + // could read the response. Only relax for a request that actually carries a + // credential, plus CORS preflights (OPTIONS never carries the header — the + // real request that follows is re-checked), so authenticated browser/Electron + // clients (#5242) keep working while credential-less cross-origin reads do not. allowed = requestOrigin && requestOrigin.length > 0 ? requestOrigin : "*"; } if (allowed !== null) { diff --git a/src/shared/constants/publicApiRoutes.ts b/src/shared/constants/publicApiRoutes.ts index 7044089a59..b34e130acd 100644 --- a/src/shared/constants/publicApiRoutes.ts +++ b/src/shared/constants/publicApiRoutes.ts @@ -71,7 +71,26 @@ function isPublicCloudApiRoute(pathname: string, method: string): boolean { ); } +// OAuth "auto-import" routes read host-local credential files (Cursor / Kiro / +// Raycast tokens). The broad `/api/oauth/` PUBLIC prefix would classify them +// PUBLIC, which skips the LOCAL_ONLY tier entirely (GHSA-wgwc-crjm-pmwv) and +// exposes the host credential to a remote caller (GHSA-gxv4-955v-v6cm). Exclude +// them so they fall through to MANAGEMENT and reach the loopback-only gate. +const LOCAL_ONLY_OAUTH_IMPORT_ROUTES = [ + "/api/oauth/cursor/auto-import", + "/api/oauth/kiro/auto-import", + "/api/oauth/raycast/auto-import", +]; + export function isPublicApiRoute(pathname: string, method = "GET"): boolean { + if ( + LOCAL_ONLY_OAUTH_IMPORT_ROUTES.some( + (route) => pathname === route || pathname.startsWith(`${route}/`) + ) + ) { + return false; + } + if (isPublicCloudApiRoute(pathname, method)) { return true; } diff --git a/src/shared/utils/apiKeyPolicy.ts b/src/shared/utils/apiKeyPolicy.ts index 7df8f1cd1d..ebd23c9ead 100644 --- a/src/shared/utils/apiKeyPolicy.ts +++ b/src/shared/utils/apiKeyPolicy.ts @@ -645,13 +645,37 @@ async function validateRateLimitAndThrottle(context: PolicyContext): Promise { - // A real bearer key wins; otherwise an authenticated dashboard playground may - // test a specific key's policy by id (resolved server-side, secret never sent). - const apiKey = extractApiKey(request) || (await resolvePlaygroundTestKey(request)); + // A real bearer key wins; then a bare x-api-key/x-goog-api-key that auth + // accepted but extractApiKey() gates out; otherwise an authenticated dashboard + // playground may test a specific key's policy by id (resolved server-side, + // secret never sent). + const apiKey = + extractApiKey(request) || + extractUngatedClientApiKey(request) || + (await resolvePlaygroundTestKey(request)); // No API key = local/session mode, skip policy checks if (!apiKey) { diff --git a/tests/unit/a2a-route-require-api-key.test.ts b/tests/unit/a2a-route-require-api-key.test.ts new file mode 100644 index 0000000000..4fec1a55c8 --- /dev/null +++ b/tests/unit/a2a-route-require-api-key.test.ts @@ -0,0 +1,69 @@ +/** + * GHSA-v54m-6rm3-p565 — /a2a sits outside the authz proxy matcher, so it never + * saw the REQUIRE_API_KEY posture and accepted every caller when OMNIROUTE_API_KEY + * was unset (the default). authenticate() now honors REQUIRE_API_KEY directly. + */ + +import test 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-require-key-")); +process.env.DATA_DIR = TEST_DATA_DIR; +process.env.API_KEY_SECRET = process.env.API_KEY_SECRET || "a2a-require-key-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 route = await import("../../src/app/a2a/route.ts"); + +const ORIGINAL_REQUIRE = process.env.REQUIRE_API_KEY; +const ORIGINAL_A2A_KEY = process.env.OMNIROUTE_API_KEY; + +test.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; + if (ORIGINAL_A2A_KEY === undefined) delete process.env.OMNIROUTE_API_KEY; + else process.env.OMNIROUTE_API_KEY = ORIGINAL_A2A_KEY; +}); + +function post(key?: string) { + return route.POST( + new Request("http://localhost/a2a", { + method: "POST", + headers: { + "content-type": "application/json", + ...(key ? { authorization: `Bearer ${key}` } : {}), + }, + body: JSON.stringify({ jsonrpc: "2.0", id: 1, method: "message/send", params: {} }), + }) as never + ); +} + +async function isUnauthorized(res: Response) { + const body = (await res.clone().json()) as { error?: { code?: number } }; + return body.error?.code === -32600; +} + +test("REQUIRE_API_KEY=true rejects an unkeyed /a2a call (GHSA-v54m)", async () => { + delete process.env.OMNIROUTE_API_KEY; + process.env.REQUIRE_API_KEY = "true"; + assert.equal(await isUnauthorized(await post()), true, "no key must be rejected"); + + const key = await apiKeysDb.createApiKey("a2a-client", "machine-a2a", []); + assert.equal( + await isUnauthorized(await post(key.key)), + false, + "a valid key must clear the /a2a auth gate" + ); +}); + +test("keyless local-first default still allows /a2a (posture preserved)", async () => { + delete process.env.REQUIRE_API_KEY; + delete process.env.OMNIROUTE_API_KEY; + assert.equal(await isUnauthorized(await post()), false, "keyless default must not 401"); +}); diff --git a/tests/unit/api-key-policy.test.ts b/tests/unit/api-key-policy.test.ts index ceda6e7348..63cf63be7b 100644 --- a/tests/unit/api-key-policy.test.ts +++ b/tests/unit/api-key-policy.test.ts @@ -96,6 +96,17 @@ function makeAnthropicPolicyRequest(apiKey) { }); } +// A bare `x-api-key` with NO anthropic-version header and no claude user-agent: +// the CLIENT_API auth layer accepts it, but the gated extractApiKey() used by +// the policy layer used to ignore it, so the key's per-key policy was skipped +// entirely (GHSA-2phc-xp22-9f56). +function makeBareXApiKeyPolicyRequest(apiKey) { + return new Request("http://localhost/v1/responses", { + method: "POST", + headers: apiKey ? { "x-api-key": apiKey } : {}, + }); +} + async function readErrorMessage(response) { const body = (await response.json()) as { error?: { message?: unknown } }; return typeof body.error?.message === "string" ? body.error.message : ""; @@ -457,6 +468,29 @@ test("enforceApiKeyPolicy rejects disabled keys and blocked schedules", async () assert.match(await readErrorMessage(blocked.rejection), /Access denied outside allowed hours/); }); +test("enforceApiKeyPolicy enforces allowedModels for a bare x-api-key (GHSA-2phc-xp22-9f56)", async () => { + const restrictedKey = await createKeyWithPolicy({ + allowedModels: ["openai/gpt-4.1"], + }); + const policy = await loadPolicy("bare-x-api-key"); + + // Disallowed model via a bare x-api-key must be rejected, exactly as it is for + // a Bearer token — the header used to carry the key must not weaken the policy. + const disallowed = await policy.enforceApiKeyPolicy( + makeBareXApiKeyPolicyRequest(restrictedKey.key), + "anthropic/claude-3-7-sonnet" + ); + assert.equal(disallowed.rejection.status, 403); + assert.match(await readErrorMessage(disallowed.rejection), /not allowed/); + + // The allowed model still passes through the same header. + const allowed = await policy.enforceApiKeyPolicy( + makeBareXApiKeyPolicyRequest(restrictedKey.key), + "openai/gpt-4.1" + ); + assert.equal(allowed.rejection, null); +}); + test("enforceApiKeyPolicy rejects disallowed models and exhausted budgets", async () => { const restrictedKey = await createKeyWithPolicy({ allowedModels: ["openai/gpt-4.1"], diff --git a/tests/unit/authz/oauth-autoimport-local-only.test.ts b/tests/unit/authz/oauth-autoimport-local-only.test.ts new file mode 100644 index 0000000000..586ad365eb --- /dev/null +++ b/tests/unit/authz/oauth-autoimport-local-only.test.ts @@ -0,0 +1,35 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { isPublicApiRoute } from "../../../src/shared/constants/publicApiRoutes.ts"; +import { classifyRoute } from "../../../src/server/authz/classify.ts"; +import { isLocalOnlyPath } from "../../../src/server/authz/routeGuard.ts"; + +// GHSA-wgwc-crjm-pmwv / GHSA-gxv4-955v-v6cm — the OAuth auto-import routes read +// host-local credential files. They must NOT be PUBLIC (which skips the LOCAL_ONLY +// tier); they must classify MANAGEMENT and be loopback-gated. + +const AUTO_IMPORT = [ + "/api/oauth/cursor/auto-import", + "/api/oauth/kiro/auto-import", + "/api/oauth/raycast/auto-import", +]; + +test("OAuth auto-import routes are excluded from PUBLIC classification", () => { + for (const p of AUTO_IMPORT) { + assert.equal(isPublicApiRoute(p), false, `${p} must not be PUBLIC`); + assert.equal(classifyRoute(p, "GET").routeClass, "MANAGEMENT", `${p} must classify MANAGEMENT`); + } +}); + +test("OAuth auto-import routes are LOCAL_ONLY (loopback-gated)", () => { + for (const p of AUTO_IMPORT) { + assert.equal(isLocalOnlyPath(p), true, `${p} must be LOCAL_ONLY`); + } +}); + +test("the rest of /api/oauth/ (callbacks, browser flows) stays PUBLIC", () => { + assert.equal(isPublicApiRoute("/api/oauth/cursor/callback"), true); + assert.equal(isPublicApiRoute("/api/oauth/codex/authorize"), true); + // A sibling that merely shares the prefix must not be swept in. + assert.equal(isPublicApiRoute("/api/oauth/cursor/auto-import-status"), true); +}); diff --git a/tests/unit/base-executor-ssrf-guard.test.ts b/tests/unit/base-executor-ssrf-guard.test.ts new file mode 100644 index 0000000000..054bd883b6 --- /dev/null +++ b/tests/unit/base-executor-ssrf-guard.test.ts @@ -0,0 +1,38 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { DefaultExecutor } from "../../open-sse/executors/default.ts"; + +// GHSA-4f49-hj64-448x — a persisted, caller-supplied providerSpecificData.baseUrl +// reaches fetch() on the runtime dispatch path with no SSRF guard. BaseExecutor +// now mirrors the provider VALIDATION guard before every upstream fetch. In the +// shipped default (block-metadata) mode the cloud-metadata IMDS pivot is blocked +// for non-local providers, public upstreams pass, and local / self-hosted +// providers (vLLM, LM Studio, Ollama, …) stay exempt so loopback/LAN keeps working. + +function guardOf(provider: string) { + const exec = new DefaultExecutor(provider) as unknown as { + assertOutboundUrlAllowed(url: string): void; + }; + return (url: string) => exec.assertOutboundUrlAllowed(url); +} + +test("BaseExecutor blocks cloud-metadata for a non-local provider (GHSA-4f49-hj64-448x)", () => { + const guard = guardOf("openai"); + assert.throws(() => guard("http://169.254.169.254/latest/meta-data/iam/security-credentials/")); + // IPv4-mapped IPv6 spelling of the same address (folded out by #10843). + assert.throws(() => guard("http://[::ffff:169.254.169.254]/latest/meta-data/")); +}); + +test("BaseExecutor allows a public upstream URL for a non-local provider", () => { + const guard = guardOf("openai"); + assert.doesNotThrow(() => guard("https://api.openai.com/v1/chat/completions")); +}); + +test("BaseExecutor exempts local / self-hosted providers from the outbound guard", () => { + assert.doesNotThrow(() => guardOf("ollama-local")("http://127.0.0.1:11434/v1/chat/completions")); + assert.doesNotThrow(() => guardOf("lm-studio")("http://192.168.1.50:1234/v1/chat/completions")); +}); + +test("BaseExecutor guard is a no-op for an empty URL", () => { + assert.doesNotThrow(() => guardOf("openai")("")); +}); diff --git a/tests/unit/cors/origins.test.ts b/tests/unit/cors/origins.test.ts index 00950aa097..7d8ecd42ec 100644 --- a/tests/unit/cors/origins.test.ts +++ b/tests/unit/cors/origins.test.ts @@ -129,12 +129,12 @@ describe("cors/origins.applyCorsHeaders", () => { assert.match(res.headers.get("Vary") || "", /Origin/); }); - it("CLIENT_API: echoes arbitrary Origin (+Vary) when no allowlist matches (relaxForTokenAuth)", () => { + it("CLIENT_API: echoes arbitrary Origin (+Vary) for a token-carrying request (relaxForTokenAuth)", () => { // Token-authenticated /v1/* surface (issue #5242): no allowlist, arbitrary // origin → echo it back so browser/Electron renderers can read the body. const res = NextResponse.json({ ok: true }); const req = new Request("https://server.example.com/api/v1/models", { - headers: { Origin: "http://localhost" }, + headers: { Origin: "http://localhost", Authorization: "Bearer omr_test_key" }, }); applyCorsHeaders(res, req, true); assert.equal(res.headers.get("Access-Control-Allow-Origin"), "http://localhost"); @@ -143,14 +143,40 @@ describe("cors/origins.applyCorsHeaders", () => { assert.equal(res.headers.get("Access-Control-Allow-Credentials"), null); }); - it("CLIENT_API: returns '*' when no Origin header is present (relaxForTokenAuth)", () => { + it("CLIENT_API: returns '*' when no Origin header is present for a token-carrying request", () => { const res = NextResponse.json({ ok: true }); - const req = new Request("https://server.example.com/api/v1/models"); + const req = new Request("https://server.example.com/api/v1/models", { + headers: { "x-api-key": "omr_test_key" }, + }); applyCorsHeaders(res, req, true); assert.equal(res.headers.get("Access-Control-Allow-Origin"), "*"); assert.equal(res.headers.get("Access-Control-Allow-Credentials"), null); }); + it("CLIENT_API: does NOT echo the Origin for a credential-less cross-origin request (GHSA-7px7)", () => { + // A keyless install serves /v1 anonymously; echoing the Origin to a + // credential-less cross-origin page would let any visited page drive the + // gateway. Only token-carrying requests get the permissive echo. + const res = NextResponse.json({ ok: true }); + const req = new Request("https://server.example.com/api/v1/models", { + headers: { Origin: "https://evil.example" }, + }); + applyCorsHeaders(res, req, true); + assert.equal(res.headers.get("Access-Control-Allow-Origin"), null); + }); + + it("CLIENT_API: a CORS preflight (OPTIONS) is still allowed through (relaxForTokenAuth)", () => { + // Preflight never carries the auth header; blocking it would break the + // credentialed request that follows, so OPTIONS keeps the permissive echo. + const res = new Response(null, { status: 204 }); + const req = new Request("https://server.example.com/api/v1/models", { + method: "OPTIONS", + headers: { Origin: "http://localhost" }, + }); + applyCorsHeaders(res, req, true); + assert.equal(res.headers.get("Access-Control-Allow-Origin"), "http://localhost"); + }); + it("MANAGEMENT: stays fail-closed for arbitrary Origin with no allowlist (relax off)", () => { const res = NextResponse.json({ ok: true }); const req = new Request("https://server.example.com/api/keys", { @@ -213,7 +239,11 @@ describe("cors/origins.applyCorsHeaders", () => { it("CLIENT_API: appends Vary: Accept-Encoding even without an Origin header (#6737)", () => { const res = NextResponse.json({ ok: true }); - const req = new Request("https://server.example.com/api/v1/models"); + // Token-carrying request (post-GHSA-7px7 the permissive echo requires a + // credential); this test's point is the Vary: Accept-Encoding stamp. + const req = new Request("https://server.example.com/api/v1/models", { + headers: { "x-api-key": "omr_test_key" }, + }); applyCorsHeaders(res, req, true); assert.equal(res.headers.get("Access-Control-Allow-Origin"), "*"); assert.match(res.headers.get("Vary") || "", /Accept-Encoding/); diff --git a/tests/unit/mcp-memory-tools-strategy.test.ts b/tests/unit/mcp-memory-tools-strategy.test.ts index a0db428663..ba740797ca 100644 --- a/tests/unit/mcp-memory-tools-strategy.test.ts +++ b/tests/unit/mcp-memory-tools-strategy.test.ts @@ -187,3 +187,37 @@ test("omniroute_memory_search: hardcoded fallback config has retrievalStrategy=e "fallback from catch path must use retrievalStrategy=exact" ); }); + +// ── IDOR: the authenticated caller's principal must win over a caller-supplied +// apiKeyId (GHSA-cpv3-xr7r-xf8q). With a resolvable caller (here: OMNIROUTE_API_KEY +// on the stdio path → "env-key"), omniroute_memory_add must store under the +// caller, NOT under the arbitrary apiKeyId in the tool arguments. +test("omniroute_memory_add: caller principal wins over a spoofed apiKeyId (GHSA-cpv3)", async () => { + const db = core.getDbInstance(); + const prevEnvKey = process.env.OMNIROUTE_API_KEY; + process.env.OMNIROUTE_API_KEY = "test-mcp-caller-key"; + try { + const { memoryTools } = await import("../../open-sse/mcp-server/tools/memoryTools.ts"); + const result = await memoryTools.omniroute_memory_add.handler({ + apiKeyId: "victim-b", + type: "factual", + key: "idor-k1", + content: "owned-by-caller", + }); + assert.equal(result.success, true, "add must succeed"); + + const rows = db + .prepare("SELECT api_key_id FROM memories WHERE key = 'idor-k1'") + .all() as Array<{ api_key_id: string }>; + assert.equal(rows.length, 1, "exactly one memory row expected"); + assert.equal( + rows[0].api_key_id, + "env-key", + "memory must be stored under the resolved caller (env-key), not the spoofed apiKeyId" + ); + assert.notEqual(rows[0].api_key_id, "victim-b", "must NOT store under the caller-supplied id"); + } finally { + if (prevEnvKey === undefined) delete process.env.OMNIROUTE_API_KEY; + else process.env.OMNIROUTE_API_KEY = prevEnvKey; + } +}); diff --git a/tests/unit/monitoring-health-public-view.test.ts b/tests/unit/monitoring-health-public-view.test.ts new file mode 100644 index 0000000000..0f8f7b2209 --- /dev/null +++ b/tests/unit/monitoring-health-public-view.test.ts @@ -0,0 +1,48 @@ +/** + * GHSA-mvf8-qc78-5mxm — GET /api/monitoring/health returned host-fingerprinting + * detail (version, node version, pid, memory, provider config) to anonymous + * callers. It now serves only the liveness verdict to non-management callers. + */ + +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import type { NextRequest } from "next/server"; +import { makeManagementSessionRequest } from "../helpers/managementSession.ts"; + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omni-health-view-")); +process.env.DATA_DIR = TEST_DATA_DIR; + +const core = await import("../../src/lib/db/core.ts"); +const route = await import("../../src/app/api/monitoring/health/route.ts"); + +test.after(() => { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); +}); + +test("anonymous health GET is reduced to liveness only (GHSA-mvf8)", async () => { + const res = await route.GET(new Request("http://localhost/api/monitoring/health") as never); + const body = (await res.json()) as Record; + assert.ok("status" in body, "liveness status must be present for probes"); + // No host fingerprinting for an anonymous caller. + const keys = Object.keys(body); + const allowed = new Set(["status", "setupComplete"]); + for (const k of keys) { + assert.ok(allowed.has(k), `anonymous health view leaked field: ${k}`); + } +}); + +test("management session sees the full health payload", async () => { + const sessionReq = (await makeManagementSessionRequest( + "http://localhost/api/monitoring/health" + )) as unknown as NextRequest; + const res = await route.GET(sessionReq as never); + const body = (await res.json()) as Record; + assert.ok( + Object.keys(body).length > 2, + "a management caller must still receive the detailed payload" + ); +}); diff --git a/tests/unit/ninerouter-embed-port-6205.test.ts b/tests/unit/ninerouter-embed-port-6205.test.ts index be967d2f53..3306bc557a 100644 --- a/tests/unit/ninerouter-embed-port-6205.test.ts +++ b/tests/unit/ninerouter-embed-port-6205.test.ts @@ -70,11 +70,19 @@ describe("#6205 A — embed panel root no longer 404s", () => { // ─── SUB-BUG B: pre-spawn port/health decision ─────────────────────────────── describe("#6205 B — pre-spawn port probe avoids raw EADDRINUSE", () => { - it("adopts a healthy existing instance (no spawn)", () => { - const decision = decidePreSpawn({ healthy: true, portInUse: true }, 20130); + it("adopts a healthy existing instance when adoption is opted in (no spawn)", () => { + const decision = decidePreSpawn({ healthy: true, portInUse: true }, 20130, true); assert.equal(decision.action, "adopt"); }); + it("does NOT adopt a healthy listener by default — a 2xx cannot prove identity (GHSA-wg9p-6m2g-4v27)", () => { + const decision = decidePreSpawn({ healthy: true, portInUse: true }, 20130); + assert.equal(decision.action, "error"); + assert.match(decision.message, /adopt/i); + assert.match(decision.message, /OMNIROUTE_ADOPT_EXISTING_SERVICE/); + assert.ok(!decision.message.includes("at /"), "must not leak a stack trace"); + }); + it("returns a clear error object (not a throw) when the port is held but unhealthy", () => { let decision; assert.doesNotThrow(() => { @@ -92,9 +100,10 @@ describe("#6205 B — pre-spawn port probe avoids raw EADDRINUSE", () => { assert.equal(decision.action, "spawn"); }); - it("adopts a healthy instance even if the TCP probe missed it", () => { - // Health is authoritative: a 2xx means a real instance is serving. - const decision = decidePreSpawn({ healthy: true, portInUse: false }, 20130); + it("adopts a healthy instance (opted in) even if the TCP probe missed it", () => { + // With adoption opted in, health is authoritative: a 2xx means a real + // instance is serving even when the TCP connect probe raced and missed it. + const decision = decidePreSpawn({ healthy: true, portInUse: false }, 20130, true); assert.equal(decision.action, "adopt"); }); }); diff --git a/tests/unit/oauth-device-code-region-ssrf.test.ts b/tests/unit/oauth-device-code-region-ssrf.test.ts new file mode 100644 index 0000000000..bc57659f79 --- /dev/null +++ b/tests/unit/oauth-device-code-region-ssrf.test.ts @@ -0,0 +1,54 @@ +/** + * GHSA-7x63-xvp5-w2jc — the kiro / amazon-q device-code action interpolates a + * caller-supplied `region` into the AWS OIDC endpoint URLs that requestDeviceCode() + * fetches. An attacker-shaped region (userinfo / fragment) re-points the outbound + * host (SSRF → cloud metadata). The route must reject a non-canonical region with + * a 400 before any outbound fetch. + */ + +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import type { NextRequest } from "next/server"; + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omni-oauth-region-ssrf-")); +process.env.DATA_DIR = TEST_DATA_DIR; + +const core = await import("../../src/lib/db/core.ts"); +const route = await import("../../src/app/api/oauth/[provider]/[action]/route.ts"); + +test.after(() => { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); +}); + +async function deviceCode(provider: string, region: string) { + const url = + `http://localhost/api/oauth/${provider}/device-code` + + `?startUrl=${encodeURIComponent("https://d-1234567890.awsapps.com/start")}` + + `®ion=${encodeURIComponent(region)}`; + return route.GET(new Request(url) as unknown as NextRequest, { + params: Promise.resolve({ provider, action: "device-code" }), + }); +} + +test("kiro device-code rejects a non-canonical region before any outbound fetch (GHSA-7x63)", async () => { + for (const bad of [ + "evil.com", + "169.254.169.254", + "us-east-1@169.254.169.254", + "us-east-1#.amazonaws.com@evil.com", + "us-east-1/../..", + "US-EAST-1", // uppercase is not the canonical shape + ]) { + const res = await deviceCode("kiro", bad); + assert.equal(res.status, 400, `region "${bad}" must be rejected with 400`); + } +}); + +test("amazon-q device-code also validates region", async () => { + const res = await deviceCode("amazon-q", "evil.com:1@169.254.169.254"); + assert.equal(res.status, 400); +}); diff --git a/tests/unit/oauth-import-manage-scope.test.ts b/tests/unit/oauth-import-manage-scope.test.ts new file mode 100644 index 0000000000..070515c02a --- /dev/null +++ b/tests/unit/oauth-import-manage-scope.test.ts @@ -0,0 +1,75 @@ +/** + * GHSA-mg76-rhpx-gvw3 / GHSA-gxv4-955v-v6cm — OAuth import / auto-import routes + * create or read provider credentials. They were guarded only by isAuthenticated(), + * which (because /api/oauth/ is PUBLIC-classified) accepts ANY valid client API key. + * They must now require MANAGEMENT scope. + */ + +import test 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-oauth-import-manage-")); +process.env.DATA_DIR = TEST_DATA_DIR; +process.env.API_KEY_SECRET = process.env.API_KEY_SECRET || "oauth-import-manage-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 settingsDb = await import("../../src/lib/db/settings.ts"); +const codexImportToken = await import("../../src/app/api/oauth/codex/import-token/route.ts"); +const cursorAutoImport = await import("../../src/app/api/oauth/cursor/auto-import/route.ts"); + +test.before(async () => { + process.env.JWT_SECRET = "oauth-import-manage-jwt"; + process.env.INITIAL_PASSWORD = "oauth-import-manage-pass"; + await settingsDb.updateSettings({ requireLogin: true }); +}); + +test.after(() => { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); + delete process.env.JWT_SECRET; + delete process.env.INITIAL_PASSWORD; +}); + +function post(route: { POST: (r: Request) => Promise }, key?: string) { + return route.POST( + new Request("http://localhost/api/oauth/codex/import-token", { + method: "POST", + headers: { + "content-type": "application/json", + ...(key ? { authorization: `Bearer ${key}` } : {}), + }, + body: JSON.stringify({ accessToken: "x", name: "poc" }), + }) + ); +} + +function get(route: { GET: (r: Request) => Promise }, key?: string) { + return route.GET( + new Request("http://localhost/api/oauth/cursor/auto-import", { + headers: key ? { authorization: `Bearer ${key}` } : {}, + }) + ); +} + +test("codex/import-token: non-manage key → 403, no key → 401, manage key passes the auth gate (GHSA-mg76)", async () => { + const nonManage = await apiKeysDb.createApiKey("client", "machine-client", []); + const manage = await apiKeysDb.createApiKey("admin", "machine-admin", ["manage"]); + + assert.equal((await post(codexImportToken, nonManage.key)).status, 403, "non-manage key rejected"); + assert.equal((await post(codexImportToken)).status, 401, "no credential rejected"); + + const withManage = await post(codexImportToken, manage.key); + assert.notEqual(withManage.status, 401, "manage key must clear the auth gate"); + assert.notEqual(withManage.status, 403, "manage key must clear the auth gate"); +}); + +test("cursor/auto-import: a non-manage key cannot read the host's Cursor token (GHSA-gxv4)", async () => { + const nonManage = await apiKeysDb.createApiKey("client2", "machine-client2", []); + assert.equal((await get(cursorAutoImport, nonManage.key)).status, 403, "non-manage key rejected"); + assert.equal((await get(cursorAutoImport)).status, 401, "no credential rejected"); +}); diff --git a/tests/unit/obsidian-webdav-route.test.ts b/tests/unit/obsidian-webdav-route.test.ts index cfc839a607..cfb9363492 100644 --- a/tests/unit/obsidian-webdav-route.test.ts +++ b/tests/unit/obsidian-webdav-route.test.ts @@ -114,7 +114,46 @@ test("POST with a valid temp dir → returns { username, password }, GET shows e const getBody = (await getRes.json()) as Record; assert.equal(getBody.webdavEnabled, true); assert.ok(typeof getBody.webdavUsername === "string" && (getBody.webdavUsername as string).length > 0); - assert.ok(typeof getBody.webdavPassword === "string" && (getBody.webdavPassword as string).length > 0); + // Anonymous GET (this request carries no management credential): the plaintext + // password is masked (GHSA-62vw), but the set/unset flag still reflects state. + assert.equal(getBody.webdavPassword, null); + assert.equal(getBody.webdavPasswordSet, true); + } finally { + fs.rmSync(vaultDir, { recursive: true, force: true }); + } +}); + +test("GET masks the WebDAV password for anonymous callers but reveals it to a management session (GHSA-62vw)", async () => { + const vaultDir = fs.mkdtempSync(path.join(os.tmpdir(), "omni-vault-62vw-")); + try { + // Enable WebDAV so there is a stored password to leak. + const enableRes = await route.POST( + makeRequest("http://localhost/api/settings/obsidian/webdav", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ vaultPath: vaultDir }), + }) + ); + assert.equal(enableRes.status, 200); + + // Anonymous (open-mode) caller: password masked, flag still set. + const anonBody = (await (await route.GET( + makeRequest("http://localhost/api/settings/obsidian/webdav") + )).json()) as Record; + assert.equal(anonBody.webdavEnabled, true); + assert.equal(anonBody.webdavPassword, null, "anonymous caller must not receive the plaintext password"); + assert.equal(anonBody.webdavPasswordSet, true); + + // Genuine management session: the operator's reveal-password view still works. + const sessionReq = (await makeManagementSessionRequest( + "http://localhost/api/settings/obsidian/webdav" + )) as unknown as NextRequest; + const sessionBody = (await (await route.GET(sessionReq)).json()) as Record; + assert.ok( + typeof sessionBody.webdavPassword === "string" && + (sessionBody.webdavPassword as string).length > 0, + "a management session must still receive the plaintext password" + ); } finally { fs.rmSync(vaultDir, { recursive: true, force: true }); }