From 2f9d9b4a2cc6ba12571aee11f89fb35e778f8bff Mon Sep 17 00:00:00 2001 From: Markus Hartung Date: Sun, 23 Aug 2026 19:00:21 -0300 Subject: [PATCH] fix(security): clear new CodeQL code-scanning alerts (round 4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - open-sse/executors/github.ts: replace the Math.random() fallback in the Copilot correlation-id generators (x-request-id, x-interaction-id, x-client-session-id, x-agent-task-id) with a CSPRNG-backed randomIdFallback() (node:crypto randomBytes) — closes js/insecure-randomness with no behavior change (crypto.randomUUID stays the primary path). - tests/unit/cli/_helpers/shellArgs.mjs: collapse the two sequential global .replace() unescape passes into a single left-to-right regex replace with alternation — closes js/double-escaping. The prior two-pass form let the first pass's output feed the second, which is exactly the double-(un)escaping bug pattern the query flags (e.g. an escaped-backslash-then-quote sequence could be misread depending on pass order). --- open-sse/executors/github.ts | 11 +++++++++-- tests/unit/cli/_helpers/shellArgs.mjs | 7 +++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/open-sse/executors/github.ts b/open-sse/executors/github.ts index f3ce1196bf..6211b509a0 100644 --- a/open-sse/executors/github.ts +++ b/open-sse/executors/github.ts @@ -1,3 +1,5 @@ +import { randomBytes } from "node:crypto"; + import { BaseExecutor, ExecuteInput, @@ -13,6 +15,11 @@ import { import { sanitizeResponsesInputItems } from "../services/responsesInputSanitizer.ts"; import { stripUnsupportedParams } from "../translator/paramSupport.ts"; +/** Correlation-id fallback for runtimes without crypto.randomUUID — still CSPRNG-backed. */ +function randomIdFallback(): string { + return `${Date.now()}-${randomBytes(9).toString("hex")}`; +} + /** * What a Copilot credential refresh resolves to. * @@ -329,7 +336,7 @@ export class GithubExecutor extends BaseExecutor { ...getGitHubCopilotChatHeaders(stream ? "text/event-stream" : "application/json", initiator), Authorization: `Bearer ${token}`, "x-request-id": - crypto.randomUUID?.() || `${Date.now()}-${Math.random().toString(36).slice(2)}`, + crypto.randomUUID?.() || randomIdFallback(), }; // Per-call / per-conversation / per-turn correlation ids the @github/copilot @@ -338,7 +345,7 @@ export class GithubExecutor extends BaseExecutor { // fresh uuids. A Copilot-aware client may pin the session/task ids across a // conversation via its own headers — honor those when present, else mint. const genId = () => - crypto.randomUUID?.() || `${Date.now()}-${Math.random().toString(36).slice(2)}`; + crypto.randomUUID?.() || randomIdFallback(); headers["x-interaction-id"] = this.readClientHeader(clientHeaders, "x-interaction-id") || genId(); headers["x-client-session-id"] = this.readClientHeader(clientHeaders, "x-client-session-id") || genId(); diff --git a/tests/unit/cli/_helpers/shellArgs.mjs b/tests/unit/cli/_helpers/shellArgs.mjs index 148fb94174..d5db0416e7 100644 --- a/tests/unit/cli/_helpers/shellArgs.mjs +++ b/tests/unit/cli/_helpers/shellArgs.mjs @@ -23,8 +23,11 @@ export function unescapeWindowsShellArg(arg) { s = s.replace(/\^(.)/g, "$1").replace(/\^(.)/g, "$1"); // 2. drop the wrapping quotes added by the CRT argv layer if (s.length >= 2 && s.startsWith('"') && s.endsWith('"')) s = s.slice(1, -1); - // 3. undo the doubled backslashes and the escaped embedded quotes - s = s.replace(/\\\\/g, "\\").replace(/\\"/g, '"'); + // 3. undo the doubled backslashes and the escaped embedded quotes in a single + // left-to-right pass — two sequential global replaces would let the first + // pass's output feed the second (e.g. an escaped-backslash-then-quote + // sequence could be misread), which is exactly what js/double-escaping flags. + s = s.replace(/\\\\|\\"/g, (m) => (m === "\\\\" ? "\\" : '"')); return s; }