Skip to content

Commit 9756e06

Browse files
galinilievGalin Iliev
galiniliev
authored and
Galin Iliev
committed
fix(github-copilot): sanitize unsafe reasoning replay ids
1 parent bc4f27c commit 9756e06

3 files changed

Lines changed: 77 additions & 13 deletions

File tree

‎extensions/github-copilot/connection-bound-ids.test.ts‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest";
22
import {
33
rewriteCopilotConnectionBoundResponseIds,
44
rewriteCopilotResponsePayloadConnectionBoundIds,
5+
sanitizeCopilotReplayResponseIds,
56
} from "./connection-bound-ids.js";
67

78
describe("github-copilot connection-bound response IDs", () => {
@@ -35,7 +36,7 @@ describe("github-copilot connection-bound response IDs", () => {
3536
expect(input[4]?.id).toMatch(/^msg_[a-f0-9]{16}$/);
3637
});
3738

38-
it("preserves reasoning IDs regardless of encrypted_content", () => {
39+
it("preserves valid reasoning IDs regardless of encrypted_content", () => {
3940
const withEncrypted = Buffer.from(`reasoning-${"e".repeat(24)}`).toString("base64");
4041
const withNull = Buffer.from(`reasoning-${"n".repeat(24)}`).toString("base64");
4142
const withoutField = Buffer.from(`reasoning-${"a".repeat(24)}`).toString("base64");
@@ -51,6 +52,38 @@ describe("github-copilot connection-bound response IDs", () => {
5152
expect(input[2]?.id).toBe(withoutField);
5253
});
5354

55+
it("preserves valid base64-ish reasoning IDs with and without encrypted content", () => {
56+
const withEncrypted = "abcDEF0123+/=";
57+
const withoutEncrypted = "reasoning/abc+123=";
58+
const input = [
59+
{ id: withEncrypted, type: "reasoning", encrypted_content: "opaque-encrypted-payload" },
60+
{ id: withoutEncrypted, type: "reasoning" },
61+
];
62+
63+
expect(sanitizeCopilotReplayResponseIds(input)).toBe(false);
64+
expect(input.map((item) => item.id)).toEqual([withEncrypted, withoutEncrypted]);
65+
});
66+
67+
it("drops unsafe reasoning replay items instead of stripping their IDs", () => {
68+
const overlongId = `5PX6gLHXT5wE+Y2tPmUV4gn+${"B".repeat(384)}`;
69+
const input = [
70+
{
71+
id: overlongId,
72+
type: "reasoning",
73+
encrypted_content: "encrypted-replay-payload",
74+
summary: [],
75+
},
76+
{ type: "reasoning", encrypted_content: "missing-id", summary: [] },
77+
{ id: 123, type: "reasoning", encrypted_content: "non-string-id", summary: [] },
78+
{ id: "rs_valid", type: "reasoning", encrypted_content: "valid", summary: [] },
79+
];
80+
81+
expect(sanitizeCopilotReplayResponseIds(input)).toBe(true);
82+
expect(input).toEqual([
83+
{ id: "rs_valid", type: "reasoning", encrypted_content: "valid", summary: [] },
84+
]);
85+
});
86+
5487
it("patches response payload input arrays only", () => {
5588
const messageId = Buffer.from(`message-${"m".repeat(24)}`).toString("base64");
5689
const payload = { input: [{ id: messageId, type: "message" }] };

‎extensions/github-copilot/connection-bound-ids.ts‎

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { createHash } from "node:crypto";
22

33
// Copilot's OpenAI-compatible `/responses` endpoint can emit replay item IDs
44
// that encode upstream connection state. Those IDs are rejected after the
5-
// connection changes, so normalize them at the provider boundary before send.
5+
// connection changes, so sanitize them at the provider boundary before send.
66

77
function looksLikeConnectionBoundId(id: string): boolean {
88
if (id.length < 24) {
@@ -25,21 +25,36 @@ function deriveReplacementId(type: string | undefined, originalId: string): stri
2525

2626
type InputItem = Record<string, unknown> & { id?: unknown; type?: unknown };
2727

28-
export function rewriteCopilotConnectionBoundResponseIds(input: unknown): boolean {
28+
function isInputItem(value: unknown): value is InputItem {
29+
return !!value && typeof value === "object";
30+
}
31+
32+
function isValidReasoningReplayId(id: unknown): id is string {
33+
return typeof id === "string" && id.length > 0 && id.length <= 64;
34+
}
35+
36+
export function sanitizeCopilotReplayResponseIds(input: unknown): boolean {
2937
if (!Array.isArray(input)) {
3038
return false;
3139
}
3240
let rewrote = false;
33-
for (const item of input as InputItem[]) {
34-
const id = item.id;
35-
if (typeof id !== "string" || id.length === 0) {
41+
for (let index = input.length - 1; index >= 0; index -= 1) {
42+
const item = input[index];
43+
if (!isInputItem(item)) {
3644
continue;
3745
}
46+
const id = item.id;
3847
// Reasoning items always reference server-side encrypted state bound to the
39-
// original item ID. Rewriting the ID — even when encrypted_content is absent
40-
// or null — breaks Copilot's server-side lookup and causes a 400 validation
41-
// failure regardless of whether the client included encrypted_content.
48+
// original item ID. Rewriting or stripping that ID can turn replay into an
49+
// invalid or ambiguous server-state lookup, so drop unsafe reasoning items.
4250
if (item.type === "reasoning") {
51+
if (!isValidReasoningReplayId(id)) {
52+
input.splice(index, 1);
53+
rewrote = true;
54+
}
55+
continue;
56+
}
57+
if (typeof id !== "string" || id.length === 0) {
4358
continue;
4459
}
4560
if (looksLikeConnectionBoundId(id)) {
@@ -50,9 +65,17 @@ export function rewriteCopilotConnectionBoundResponseIds(input: unknown): boolea
5065
return rewrote;
5166
}
5267

53-
export function rewriteCopilotResponsePayloadConnectionBoundIds(payload: unknown): boolean {
68+
export function rewriteCopilotConnectionBoundResponseIds(input: unknown): boolean {
69+
return sanitizeCopilotReplayResponseIds(input);
70+
}
71+
72+
export function sanitizeCopilotReplayResponsePayloadIds(payload: unknown): boolean {
5473
if (!payload || typeof payload !== "object") {
5574
return false;
5675
}
57-
return rewriteCopilotConnectionBoundResponseIds((payload as { input?: unknown }).input);
76+
return sanitizeCopilotReplayResponseIds((payload as { input?: unknown }).input);
77+
}
78+
79+
export function rewriteCopilotResponsePayloadConnectionBoundIds(payload: unknown): boolean {
80+
return sanitizeCopilotReplayResponsePayloadIds(payload);
5881
}

‎extensions/github-copilot/stream.test.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,14 +118,21 @@ describe("wrapCopilotAnthropicStream", () => {
118118
expect(baseStreamFn.mock.calls).toEqual([[model, context, options]]);
119119
});
120120

121-
it("adds Copilot headers, preserves reasoning IDs, and rewrites message IDs before payload send", () => {
121+
it("adds Copilot headers, sanitizes reasoning replay, and rewrites message IDs before payload send", () => {
122122
const reasoningId = Buffer.from(`reasoning-${"x".repeat(24)}`).toString("base64");
123+
const overlongReasoningId = `5PX6gLHXT5wE+Y2tPmUV4gn+${"B".repeat(384)}`;
123124
const messageId = Buffer.from(`message-${"y".repeat(24)}`).toString("base64");
124125
const payloads: Array<{ input: Array<Record<string, unknown>> }> = [];
125126
const baseStreamFn = vi.fn((_model, _context, options) => {
126127
const payload = {
127128
input: [
128-
{ id: reasoningId, type: "reasoning" },
129+
{ id: reasoningId, type: "reasoning", encrypted_content: "valid-encrypted-payload" },
130+
{
131+
id: overlongReasoningId,
132+
type: "reasoning",
133+
encrypted_content: "invalid-encrypted-payload",
134+
summary: [],
135+
},
129136
{ id: messageId, type: "message" },
130137
],
131138
};
@@ -174,6 +181,7 @@ describe("wrapCopilotAnthropicStream", () => {
174181
onPayload: options.onPayload,
175182
});
176183
expect(payloads[0]?.input[0]?.id).toBe(reasoningId);
184+
expect(payloads[0]?.input.map((item) => item.type)).toEqual(["reasoning", "message"]);
177185
expect(payloads[0]?.input[1]?.id).toMatch(/^msg_[a-f0-9]{16}$/);
178186
});
179187

0 commit comments

Comments
 (0)