From ad55d486cec3d4fb560447c4fcae700d8eaf216a Mon Sep 17 00:00:00 2001 From: Galin Iliev Date: Sun, 17 May 2026 19:48:27 -0700 Subject: [PATCH] fix(github-copilot): sanitize unsafe reasoning replay ids (#83221) Fixes #83220. --- CHANGELOG.md | 1 + .../connection-bound-ids.test.ts | 35 ++++++++++++++- .../github-copilot/connection-bound-ids.ts | 43 ++++++++++++++----- extensions/github-copilot/stream.test.ts | 12 +++++- 4 files changed, 78 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7684f5f8c85c..17b7645beb13 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- GitHub Copilot: drop unsafe native Responses reasoning replay items with non-replayable IDs before dispatch, preventing affected Copilot sessions from failing with `invalid_request_body`. Fixes #83220. Thanks @galiniliev. - Core/plugins: harden clawpatch-reported edge cases across gateway auth cleanup, Claude session id paths, plugin activation policy, apply-patch hunk handling, diagnostic redaction, and plugin metadata validation. - Mac app: keep app-level menu commands and Dashboard failure states reachable when the remote Gateway is disconnected, and keep the Settings sidebar toggle in the leading titlebar area. - Gateway/webchat: hide internal runtime-context and other `display: false` transcript messages from Chat history and live message events. Fixes #83216. Thanks @EmpireCreator. diff --git a/extensions/github-copilot/connection-bound-ids.test.ts b/extensions/github-copilot/connection-bound-ids.test.ts index 57bafe239ded..7827c189bfe1 100644 --- a/extensions/github-copilot/connection-bound-ids.test.ts +++ b/extensions/github-copilot/connection-bound-ids.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest"; import { rewriteCopilotConnectionBoundResponseIds, rewriteCopilotResponsePayloadConnectionBoundIds, + sanitizeCopilotReplayResponseIds, } from "./connection-bound-ids.js"; describe("github-copilot connection-bound response IDs", () => { @@ -35,7 +36,7 @@ describe("github-copilot connection-bound response IDs", () => { expect(input[4]?.id).toMatch(/^msg_[a-f0-9]{16}$/); }); - it("preserves reasoning IDs regardless of encrypted_content", () => { + it("preserves valid reasoning IDs regardless of encrypted_content", () => { const withEncrypted = Buffer.from(`reasoning-${"e".repeat(24)}`).toString("base64"); const withNull = Buffer.from(`reasoning-${"n".repeat(24)}`).toString("base64"); const withoutField = Buffer.from(`reasoning-${"a".repeat(24)}`).toString("base64"); @@ -51,6 +52,38 @@ describe("github-copilot connection-bound response IDs", () => { expect(input[2]?.id).toBe(withoutField); }); + it("preserves valid base64-ish reasoning IDs with and without encrypted content", () => { + const withEncrypted = "abcDEF0123+/="; + const withoutEncrypted = "reasoning/abc+123="; + const input = [ + { id: withEncrypted, type: "reasoning", encrypted_content: "opaque-encrypted-payload" }, + { id: withoutEncrypted, type: "reasoning" }, + ]; + + expect(sanitizeCopilotReplayResponseIds(input)).toBe(false); + expect(input.map((item) => item.id)).toEqual([withEncrypted, withoutEncrypted]); + }); + + it("drops unsafe reasoning replay items instead of stripping their IDs", () => { + const overlongId = `5PX6gLHXT5wE+Y2tPmUV4gn+${"B".repeat(384)}`; + const input = [ + { + id: overlongId, + type: "reasoning", + encrypted_content: "encrypted-replay-payload", + summary: [], + }, + { type: "reasoning", encrypted_content: "missing-id", summary: [] }, + { id: 123, type: "reasoning", encrypted_content: "non-string-id", summary: [] }, + { id: "rs_valid", type: "reasoning", encrypted_content: "valid", summary: [] }, + ]; + + expect(sanitizeCopilotReplayResponseIds(input)).toBe(true); + expect(input).toEqual([ + { id: "rs_valid", type: "reasoning", encrypted_content: "valid", summary: [] }, + ]); + }); + it("patches response payload input arrays only", () => { const messageId = Buffer.from(`message-${"m".repeat(24)}`).toString("base64"); const payload = { input: [{ id: messageId, type: "message" }] }; diff --git a/extensions/github-copilot/connection-bound-ids.ts b/extensions/github-copilot/connection-bound-ids.ts index f5cf6d60bb18..cfd9f839f1f8 100644 --- a/extensions/github-copilot/connection-bound-ids.ts +++ b/extensions/github-copilot/connection-bound-ids.ts @@ -2,7 +2,7 @@ import { createHash } from "node:crypto"; // Copilot's OpenAI-compatible `/responses` endpoint can emit replay item IDs // that encode upstream connection state. Those IDs are rejected after the -// connection changes, so normalize them at the provider boundary before send. +// connection changes, so sanitize them at the provider boundary before send. function looksLikeConnectionBoundId(id: string): boolean { if (id.length < 24) { @@ -25,21 +25,36 @@ function deriveReplacementId(type: string | undefined, originalId: string): stri type InputItem = Record & { id?: unknown; type?: unknown }; -export function rewriteCopilotConnectionBoundResponseIds(input: unknown): boolean { +function isInputItem(value: unknown): value is InputItem { + return !!value && typeof value === "object"; +} + +function isValidReasoningReplayId(id: unknown): id is string { + return typeof id === "string" && id.length > 0 && id.length <= 64; +} + +export function sanitizeCopilotReplayResponseIds(input: unknown): boolean { if (!Array.isArray(input)) { return false; } let rewrote = false; - for (const item of input as InputItem[]) { - const id = item.id; - if (typeof id !== "string" || id.length === 0) { + for (let index = input.length - 1; index >= 0; index -= 1) { + const item = input[index]; + if (!isInputItem(item)) { continue; } + const id = item.id; // Reasoning items always reference server-side encrypted state bound to the - // original item ID. Rewriting the ID — even when encrypted_content is absent - // or null — breaks Copilot's server-side lookup and causes a 400 validation - // failure regardless of whether the client included encrypted_content. + // original item ID. Rewriting or stripping that ID can turn replay into an + // invalid or ambiguous server-state lookup, so drop unsafe reasoning items. if (item.type === "reasoning") { + if (!isValidReasoningReplayId(id)) { + input.splice(index, 1); + rewrote = true; + } + continue; + } + if (typeof id !== "string" || id.length === 0) { continue; } if (looksLikeConnectionBoundId(id)) { @@ -50,9 +65,17 @@ export function rewriteCopilotConnectionBoundResponseIds(input: unknown): boolea return rewrote; } -export function rewriteCopilotResponsePayloadConnectionBoundIds(payload: unknown): boolean { +export function rewriteCopilotConnectionBoundResponseIds(input: unknown): boolean { + return sanitizeCopilotReplayResponseIds(input); +} + +export function sanitizeCopilotReplayResponsePayloadIds(payload: unknown): boolean { if (!payload || typeof payload !== "object") { return false; } - return rewriteCopilotConnectionBoundResponseIds((payload as { input?: unknown }).input); + return sanitizeCopilotReplayResponseIds((payload as { input?: unknown }).input); +} + +export function rewriteCopilotResponsePayloadConnectionBoundIds(payload: unknown): boolean { + return sanitizeCopilotReplayResponsePayloadIds(payload); } diff --git a/extensions/github-copilot/stream.test.ts b/extensions/github-copilot/stream.test.ts index abfe148c69ae..63c862d8a947 100644 --- a/extensions/github-copilot/stream.test.ts +++ b/extensions/github-copilot/stream.test.ts @@ -118,14 +118,21 @@ describe("wrapCopilotAnthropicStream", () => { expect(baseStreamFn.mock.calls).toEqual([[model, context, options]]); }); - it("adds Copilot headers, preserves reasoning IDs, and rewrites message IDs before payload send", () => { + it("adds Copilot headers, sanitizes reasoning replay, and rewrites message IDs before payload send", () => { const reasoningId = Buffer.from(`reasoning-${"x".repeat(24)}`).toString("base64"); + const overlongReasoningId = `5PX6gLHXT5wE+Y2tPmUV4gn+${"B".repeat(384)}`; const messageId = Buffer.from(`message-${"y".repeat(24)}`).toString("base64"); const payloads: Array<{ input: Array> }> = []; const baseStreamFn = vi.fn((_model, _context, options) => { const payload = { input: [ - { id: reasoningId, type: "reasoning" }, + { id: reasoningId, type: "reasoning", encrypted_content: "valid-encrypted-payload" }, + { + id: overlongReasoningId, + type: "reasoning", + encrypted_content: "invalid-encrypted-payload", + summary: [], + }, { id: messageId, type: "message" }, ], }; @@ -174,6 +181,7 @@ describe("wrapCopilotAnthropicStream", () => { onPayload: options.onPayload, }); expect(payloads[0]?.input[0]?.id).toBe(reasoningId); + expect(payloads[0]?.input.map((item) => item.type)).toEqual(["reasoning", "message"]); expect(payloads[0]?.input[1]?.id).toMatch(/^msg_[a-f0-9]{16}$/); });