From 058cfd4f9590587b8db97111e62916bf4e8ea693 Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> Date: Thu, 2 Jul 2026 17:29:16 -0300 Subject: [PATCH] fix(sse): strip ANSI/VT100 codes from gemini-cli stream frames (#5934) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Integrated into release/v3.8.44 — ReDoS-safe ANSI/VT100 strip for gemini-cli stream frames (port of upstream #2273, thanks @anki1kr). PR test green (5/5), file-size gate OK. --- CHANGELOG.md | 6 +-- .../translator/response/gemini-to-openai.ts | 15 ++++-- open-sse/utils/streamHelpers.ts | 36 +++++++++++++-- .../unit/gemini-cli-ansi-sanitization.test.ts | 46 +++++++++++++++++++ 4 files changed, 90 insertions(+), 13 deletions(-) create mode 100644 tests/unit/gemini-cli-ansi-sanitization.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 7dc8921677..57375278ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,13 +12,11 @@ _TBD_ ### 🔧 Bug Fixes -- **fix(providers): Api Airforce model discovery no longer produces a doubled `/v1` path** — a base URL ending in `/v1/chat/completions` (e.g. `https://api.airforce/v1/chat/completions`) was only stripped of `/chat/completions`, leaving a trailing `/v1` that the endpoint builder then doubled into `…/v1/v1/models`. That 308 redirect was surfaced as `REDIRECT_BLOCKED` and aborted the whole discovery probe loop before the correct `…/v1/models` candidate. The `/v1` suffix is now stripped independently (guarding a host literally named `v1`), and a `REDIRECT_BLOCKED` on one candidate continues to the next endpoint instead of aborting. Regression guards: `tests/unit/provider-models-route.test.ts`. ([#5904](https://github.com/diegosouzapw/OmniRoute/pull/5904) — thanks [@hamsa0x7](https://github.com/hamsa0x7)). Also reported/fixed independently by [@anki1kr](https://github.com/anki1kr) in [#5920](https://github.com/diegosouzapw/OmniRoute/pull/5920). +- **fix(sse):** strip ANSI/VT100 escape codes from gemini-cli stream frames so ANSI-prefixed `data:` lines are no longer silently dropped. (thanks @anki1kr) ### 📝 Maintenance -- **chore(release):** release-pipeline hardening — `check:test-masking` (vs `origin/main`) is now a HARD gate in the release-green pre-flight (`validate-release-green.mjs`), so a non-allowlisted net-assert reduction surfaces locally instead of in a ~40-min CI layer on the release PR; plus two reconciliation helpers — `npm run release:contributors` (reproducible `### 🙌 Contributors` table via a parenthetical-group parser) and `npm run release:uncovered` (lists commits with no CHANGELOG bullet). ([#5926](https://github.com/diegosouzapw/OmniRoute/pull/5926) — thanks @diegosouzapw) - -- **chore(ci):** the `check:pr-evidence` FAIL report now tells you that editing the PR body does not re-run the gate (`ci.yml` ignores the `edited` event) — push a commit to re-validate. ([#5944](https://github.com/diegosouzapw/OmniRoute/pull/5944) — thanks @diegosouzapw) +_TBD_ --- diff --git a/open-sse/translator/response/gemini-to-openai.ts b/open-sse/translator/response/gemini-to-openai.ts index dbf608b148..9d8a8629fe 100644 --- a/open-sse/translator/response/gemini-to-openai.ts +++ b/open-sse/translator/response/gemini-to-openai.ts @@ -9,6 +9,7 @@ import { containsTextualToolCallMarker, } from "../../utils/textualToolCall.ts"; import { normalizeOpenAICompatibleFinishReasonString } from "../../utils/finishReason.ts"; +import { stripAnsiCodes } from "../../utils/streamHelpers.ts"; type GeminiToOpenAIState = { functionIndex: number; @@ -401,6 +402,10 @@ export function geminiToOpenAIResponse(chunk, state) { // Process parts if (content?.parts) { for (const part of content.parts) { + // Normalize the part text once: strip ANSI/VT100 escape codes that some + // upstreams (gemini-cli terminal redraws) inject, so the `` / + // `[Tool call:]` textual parsers below never see stray control bytes (#2273). + const partText = stripAnsiCodes(part.text); const hasThoughtSig = part.thoughtSignature || part.thought_signature; const isThought = part.thought === true; if (hasThoughtSig && typeof hasThoughtSig === "string") { @@ -409,7 +414,7 @@ export function geminiToOpenAIResponse(chunk, state) { // Handle thought signature (thinking mode) or native gemini thought flag if (hasThoughtSig || isThought) { - const hasTextContent = part.text !== undefined && part.text !== ""; + const hasTextContent = partText !== undefined && partText !== ""; const hasFunctionCall = !!part.functionCall; // Gemini/Antigravity can emit thoughtSignature as a standalone part @@ -433,7 +438,7 @@ export function geminiToOpenAIResponse(chunk, state) { choices: [ { index: 0, - delta: isThought ? { reasoning_content: part.text } : { content: part.text }, + delta: isThought ? { reasoning_content: partText } : { content: partText }, finish_reason: null, }, ], @@ -463,10 +468,10 @@ export function geminiToOpenAIResponse(chunk, state) { // "[Tool call: ...]" block instead of native functionCall. Convert that // back to a structured OpenAI tool call so clients/tools do not see it as // assistant prose. - if (part.text !== undefined && part.text !== "") { + if (partText !== undefined && partText !== "") { const afterReasoning = parseTextualReasoningTags - ? consumeTextualReasoningTags(part.text, state, results) - : part.text; + ? consumeTextualReasoningTags(partText, state, results) + : partText; if (!afterReasoning) continue; let accumulated = (state.textualToolCallBuffer || "") + afterReasoning; diff --git a/open-sse/utils/streamHelpers.ts b/open-sse/utils/streamHelpers.ts index bfa5a2aad2..03e0b66f5c 100644 --- a/open-sse/utils/streamHelpers.ts +++ b/open-sse/utils/streamHelpers.ts @@ -53,6 +53,31 @@ function isRecord(value: unknown): value is Record { return !!value && typeof value === "object" && !Array.isArray(value); } +/** + * Matches ANSI/VT100 terminal control sequences plus non-whitespace C0 control + * codes, while preserving `\t` (0x09), `\n` (0x0a), and `\r` (0x0d). + * + * Some upstream CLIs (notably gemini-cli via the `gc/` bridge) prefix SSE frames + * with cursor-movement escapes such as `\x1b[2K\x1b[1A` to redraw the terminal. + * Those bytes are not whitespace, so `line.trimStart().startsWith("data:")` fails + * and the frame is silently dropped, stalling the client SSE parser (issue #2273). + * + * The pattern is strictly bounded (no unbounded quantifiers over overlapping + * alternatives) so it runs in linear time on untrusted input — ReDoS-safe. + */ +// eslint-disable-next-line no-control-regex +const ANSI_ESCAPE_RE = + /\x1b(?:\[[0-9;?]*[A-Za-z]|\][^\x07\x1b]*(?:\x07|\x1b\\)|[A-Z\[\]\\^_`])|[\x00-\x08\x0b\x0c\x0e-\x1f]/g; + +/** + * Strip ANSI/VT100 escape sequences (and stray C0 controls) from a string. + * Non-string inputs (null/undefined) are returned unchanged. Preserves \t \n \r. + */ +export function stripAnsiCodes(str: T): T { + if (typeof str !== "string") return str; + return str.replace(ANSI_ESCAPE_RE, "") as T; +} + export function parseSSEDataPayload( data: unknown, options: SSEPayloadOptions = {} @@ -88,15 +113,18 @@ export function parseSSEDataLines( export function parseSSELine(line: string): SSEJsonPayload | null { if (!line) return null; - // Trim leading whitespace before checking field name. + // Trim leading whitespace before checking field name. Also strip ANSI/VT100 + // escape codes so terminal-redraw-prefixed frames (e.g. gemini-cli `\x1b[2K\x1b[1A`) + // still resolve to a `data:` line instead of being silently dropped (#2273). const trimmed = line.trimStart(); - if (!trimmed.startsWith("data:")) return null; + const clean = stripAnsiCodes(trimmed); + if (!clean.startsWith("data:")) return null; - return parseSSEDataPayload(trimmed.slice(5)); + return parseSSEDataPayload(clean.slice(5)); } function extractSseDataLine(line: string): string | null { - const trimmed = line.trimStart().replace(/\r$/, ""); + const trimmed = stripAnsiCodes(line.trimStart().replace(/\r$/, "")); if (!trimmed.startsWith("data:")) return null; return trimmed.slice(5).trimStart(); } diff --git a/tests/unit/gemini-cli-ansi-sanitization.test.ts b/tests/unit/gemini-cli-ansi-sanitization.test.ts new file mode 100644 index 0000000000..0291be48f7 --- /dev/null +++ b/tests/unit/gemini-cli-ansi-sanitization.test.ts @@ -0,0 +1,46 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; + +import { parseSSELine, stripAnsiCodes } from "../../open-sse/utils/streamHelpers.ts"; + +test("parseSSELine resolves an ANSI/VT100-prefixed data: frame (gemini-cli redraw)", () => { + // gemini-cli prefixes SSE frames with cursor-redraw escapes (\x1b[2K clears the + // line, \x1b[1A moves the cursor up). 0x1b is not whitespace, so before the fix + // trimStart().startsWith("data:") failed and the frame was silently dropped (#2273). + const line = `\x1b[2K\x1b[1Adata: ${JSON.stringify({ + choices: [{ delta: { content: "hi" } }], + })}`; + const r = parseSSELine(line); + assert.ok(r, "expected a parsed payload, got null (frame was dropped)"); + assert.equal(r?.choices?.[0]?.delta?.content, "hi"); +}); + +test("parseSSELine returns null for a pure-ANSI line (nothing after stripping)", () => { + assert.equal(parseSSELine("\x1b[2K\x1b[1A"), null); +}); + +test("stripAnsiCodes strips CSI/SGR/OSC/C0 but preserves \\t \\n \\r", () => { + // CSI cursor moves + SGR color codes + assert.equal(stripAnsiCodes("\x1b[2K\x1b[1Ahello"), "hello"); + assert.equal(stripAnsiCodes("\x1b[31mred\x1b[0m"), "red"); + // OSC sequence terminated by BEL (\x07) + assert.equal(stripAnsiCodes("\x1b]0;title\x07text"), "text"); + // OSC sequence terminated by ST (\x1b\\) + assert.equal(stripAnsiCodes("\x1b]8;;https://x\x1b\\link"), "link"); + // stray C0 control byte + assert.equal(stripAnsiCodes("a\x00b"), "ab"); + // whitespace preserved + assert.equal(stripAnsiCodes("a\tb\nc\rd"), "a\tb\nc\rd"); +}); + +test("stripAnsiCodes passes null/undefined through unchanged", () => { + assert.equal(stripAnsiCodes(null), null); + assert.equal(stripAnsiCodes(undefined), undefined); +}); + +test("stripAnsiCodes runs in linear time on adversarial input (ReDoS guard)", () => { + const hostile = "\x1b[" + "0;".repeat(50000) + "m"; + const start = Date.now(); + stripAnsiCodes(hostile); + assert.ok(Date.now() - start < 1000, "stripAnsiCodes should not backtrack catastrophically"); +});