From fec6164e9257cb567b0100301016879fa415ada0 Mon Sep 17 00:00:00 2001 From: diegosouzapw Date: Mon, 18 May 2026 23:27:34 -0300 Subject: [PATCH] fix(security): resolve CodeQL alerts #243/#244/#245 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #243 (js/request-forgery, high) — providers/bulk/route.ts - Replace `fetch(\${origin}/api/providers/validate)` (where origin came from spoofable `new URL(request.url).origin`) with a direct in-process call to validateProviderApiKey. Eliminates the SSRF vector and the HTTP round-trip through the same app. - Resolve proxy once outside the loop and reuse via runWithProxyContext. - Drop now-unused passthroughAuthHeaders helper. #244 (js/resource-exhaustion, warn) — copilot-web.ts::solveHashcash - Clamp upstream-supplied `difficulty` to [1, 8] before `"0".repeat(difficulty)` so a malicious/buggy server can't force a huge prefix allocation or push the 10M-iteration loop into effectively unbounded work. #245 (js/insufficient-password-hash, warn) — copilot-web.ts::getSession - Dedupe the inline `createHash("sha256").update(accessToken)` call by reusing the existing sessionPoolKey helper. - Rename its parameter from `accessToken` to `token` and document that the input is a high-entropy OAuth bearer used only as an in-memory Map key — bcrypt/scrypt/argon2 would be incorrect here, and SHA-256:16 is an appropriate fingerprint per docs/security/PUBLIC_CREDS.md. Tests - Export solveHashcash and add unit tests asserting it returns null for out-of-range / non-integer difficulty and produces a numeric nonce for the common difficulty=1 case. - All 26 tests in copilot-web-executor.test.ts and providers-bulk-route.test.ts continue to pass; sessionPoolKey contract (SHA-256:16) preserved. --- open-sse/executors/copilot-web.ts | 34 ++++++++++++++----- src/app/api/providers/bulk/route.ts | 43 +++++++++++++------------ tests/unit/copilot-web-executor.test.ts | 20 +++++++++++- 3 files changed, 67 insertions(+), 30 deletions(-) diff --git a/open-sse/executors/copilot-web.ts b/open-sse/executors/copilot-web.ts index e50a0b8af7..10d51212d2 100644 --- a/open-sse/executors/copilot-web.ts +++ b/open-sse/executors/copilot-web.ts @@ -74,7 +74,16 @@ export function getCopilotMode(model?: string): string { return MODEL_MODE_MAP[lower] || DEFAULT_MODE; } -function solveHashcash(parameter: string, difficulty: number): number | null { +// Hashcash difficulty cap. Upstream supplies `difficulty`, so we clamp it to +// prevent a malicious/buggy server from forcing huge prefix allocations or +// effectively infinite work. 8 hex zeros = 2^32 expected iterations, already +// far beyond the ~10M iteration budget below. +const MAX_HASHCASH_DIFFICULTY = 8; + +export function solveHashcash(parameter: string, difficulty: number): number | null { + if (!Number.isInteger(difficulty) || difficulty < 1 || difficulty > MAX_HASHCASH_DIFFICULTY) { + return null; + } const prefix = "0".repeat(difficulty); for (let i = 0; i < 10_000_000; i++) { const hash = createHash("sha256").update(`${parameter}:${i}`).digest("hex"); @@ -96,10 +105,21 @@ export function extractAccessToken(credential: string): string | null { return credential; } -export function sessionPoolKey(accessToken?: string): string { - return accessToken - ? createHash("sha256").update(accessToken).digest("hex").slice(0, 16) - : "anonymous"; +/** + * Compute an in-memory session-pool fingerprint for an OAuth access token. + * + * The input is a high-entropy bearer token (not a user password), and the + * output is only used as a Map key for in-process session reuse — it never + * leaves the process, is never persisted, and is never compared against + * untrusted input. SHA-256 truncated to 16 hex chars is therefore an + * appropriate cryptographic fingerprint: bcrypt/scrypt/argon2 would be + * incorrect here, since their slowness exists to thwart brute-force of + * low-entropy human secrets we do not have. See docs/security/PUBLIC_CREDS.md + * for the broader credential-handling pattern. + */ +export function sessionPoolKey(token?: string): string { + if (!token) return "anonymous"; + return createHash("sha256").update(token).digest("hex").slice(0, 16); } // ─── Session Management ───────────────────────────────────────────────────── @@ -130,9 +150,7 @@ export class CopilotWebExecutor extends BaseExecutor { * Get or create a session. Rotates when remainingTurns is low or blocked. */ private async getSession(accessToken?: string, signal?: AbortSignal): Promise { - const poolKey = accessToken - ? createHash("sha256").update(accessToken).digest("hex").slice(0, 16) - : "anonymous"; + const poolKey = sessionPoolKey(accessToken); const existing = sessionPool.get(poolKey); if ( diff --git a/src/app/api/providers/bulk/route.ts b/src/app/api/providers/bulk/route.ts index 799b205ca1..ce8e2aa491 100644 --- a/src/app/api/providers/bulk/route.ts +++ b/src/app/api/providers/bulk/route.ts @@ -21,6 +21,9 @@ import { import { requireManagementAuth } from "@/lib/api/requireManagementAuth"; import { isManagedProviderConnectionId } from "@/lib/providers/catalog"; import { sanitizeErrorMessage } from "@omniroute/open-sse/utils/error"; +import { validateProviderApiKey } from "@/lib/providers/validation"; +import { getProxyForLevel, resolveProxyForProvider } from "@/lib/localDb"; +import { runWithProxyContext } from "@omniroute/open-sse/utils/proxyFetch.ts"; // POST /api/providers/bulk — create multiple API-key connections for a single provider. // Partial-failure semantics: each entry succeeds or fails independently; the @@ -89,7 +92,17 @@ export async function POST(request: Request) { baseProviderSpecificData = normalizeProviderSpecificData(provider, baseProviderSpecificData) || null; - const origin = new URL(request.url).origin; + // Resolve proxy once for all entries — we call validateProviderApiKey directly + // instead of round-tripping through /api/providers/validate over HTTP. Direct + // invocation avoids SSRF risk from `new URL(request.url).origin` being driven + // by a spoofable Host header (CodeQL js/request-forgery #243). + const proxyToUse = validateKeys + ? (await resolveProxyForProvider(provider)) || + (await getProxyForLevel("provider", provider)) || + (await getProxyForLevel("global")) || + null + : null; + const created: Array> = []; const errors: Array<{ index: number; name: string; message: string }> = []; @@ -99,17 +112,14 @@ export async function POST(request: Request) { let testStatus: "active" | "unknown" | "failed" = "unknown"; if (validateKeys) { - const probe = await fetch(`${origin}/api/providers/validate`, { - method: "POST", - headers: { - "Content-Type": "application/json", - // Forward auth so the validate endpoint accepts the call. - ...passthroughAuthHeaders(request), - }, - body: JSON.stringify({ provider, apiKey: entry.apiKey }), - }); - const probeData = (await probe.json().catch(() => ({}))) as { valid?: boolean }; - testStatus = probeData.valid ? "active" : "failed"; + const probe = await runWithProxyContext(proxyToUse, () => + validateProviderApiKey({ + provider, + apiKey: entry.apiKey, + providerSpecificData: baseProviderSpecificData || {}, + }) + ); + testStatus = probe?.valid ? "active" : "failed"; } const newConnection = await createProviderConnection({ @@ -188,15 +198,6 @@ export async function POST(request: Request) { ); } -function passthroughAuthHeaders(request: Request): Record { - const out: Record = {}; - const auth = request.headers.get("authorization"); - if (auth) out.authorization = auth; - const cookie = request.headers.get("cookie"); - if (cookie) out.cookie = cookie; - return out; -} - async function syncToCloudIfEnabled() { try { const cloudEnabled = await isCloudEnabled(); diff --git a/tests/unit/copilot-web-executor.test.ts b/tests/unit/copilot-web-executor.test.ts index 5920b27348..ad00624469 100644 --- a/tests/unit/copilot-web-executor.test.ts +++ b/tests/unit/copilot-web-executor.test.ts @@ -2,7 +2,7 @@ import test from "node:test"; import assert from "node:assert/strict"; import { createHash } from "node:crypto"; -const { getCopilotMode, extractAccessToken, sessionPoolKey } = +const { getCopilotMode, extractAccessToken, sessionPoolKey, solveHashcash } = await import("../../open-sse/executors/copilot-web.ts"); test("getCopilotMode maps known models to their Copilot modes", () => { @@ -71,3 +71,21 @@ test("sessionPoolKey is a 16-char hex prefix of sha256", () => { assert.equal(sessionPoolKey(token), expected); assert.match(sessionPoolKey(token), /^[0-9a-f]{16}$/); }); + +// solveHashcash difficulty bounds — CodeQL js/resource-exhaustion #244 guard. +test("solveHashcash rejects out-of-range difficulty to avoid resource exhaustion", () => { + // Negative, zero, fractional, NaN, Infinity, and >8 must short-circuit. + assert.equal(solveHashcash("param", 0), null); + assert.equal(solveHashcash("param", -1), null); + assert.equal(solveHashcash("param", 1.5), null); + assert.equal(solveHashcash("param", Number.NaN), null); + assert.equal(solveHashcash("param", Number.POSITIVE_INFINITY), null); + assert.equal(solveHashcash("param", 9), null); + assert.equal(solveHashcash("param", 1_000_000), null); +}); + +test("solveHashcash succeeds for difficulty=1 (a single leading zero is common)", () => { + // ~1 in 16 chance of leading "0" — well within the 10M iteration budget. + const result = solveHashcash("any-parameter", 1); + assert.ok(typeof result === "number" && result >= 0, "expected a numeric nonce"); +});