Skip to content

fix(adapters): reject malformed nested response shapes #1332

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Ingwannu wants to merge 1 commit into from
+117 −13
Closed

fix(adapters): reject malformed nested response shapes #1332

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 22 additions & 5 deletions src/adapters/google.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -577,15 +577,32 @@ export function createGoogleAdapter(provider: OcxProviderConfig): ProviderAdapte
pendingUsage = usageFromGemini(usageMeta);
sawTerminalSignal = true;
}
const candidates = root.candidates as { content?: { parts?: unknown[] }; finishReason?: string }[] | undefined;
if (!candidates?.length) return "continue";
const rawCandidates = root.candidates;
if (rawCandidates === undefined) return "continue";
if (!Array.isArray(rawCandidates)) {
yield { type: "error", message: "google response contained invalid candidates" };
return "terminate";
}
if (rawCandidates.length === 0) return "continue";
const rawCandidate = rawCandidates[0];
if (rawCandidate === null || typeof rawCandidate !== "object" || Array.isArray(rawCandidate)) {
// Unlike a root `data: null` keepalive, this is a claimed response candidate. Treat it
// as terminal protocol corruption so the turn cannot complete after silently losing
// a candidate or tool call (#1325).
yield { type: "error", message: "google response contained invalid candidates" };
return "terminate";
}
const candidate = rawCandidate as {
content?: { parts?: unknown[] };
finishReason?: string;
};

if (typeof candidates[0].finishReason === "string" && candidates[0].finishReason) {
lastFinishReason = candidates[0].finishReason;
if (typeof candidate.finishReason === "string" && candidate.finishReason) {
lastFinishReason = candidate.finishReason;
sawTerminalSignal = true;
}

const parts = candidates[0].content?.parts as { text?: string; functionCall?: { name: string; args: unknown } }[] | undefined;
const parts = candidate.content?.parts as { text?: string; functionCall?: { name: string; args: unknown } }[] | undefined;

@coderabbitai coderabbitai Bot Aug 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Validate candidate.content.parts before iterating.

The candidate guard does not validate content.parts. A frame with parts: {} throws at Line 615. A frame with parts: [null] throws at Line 616. The outer catch rethrows these errors because they are not translator-budget errors.

Allow absent content or parts frames. If parts is present, reject a non-array container or non-object part through the terminal adapter error channel. Add regression cases for both shapes.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/adapters/google.ts` at line 605, Update the candidate processing around
the parts variable and its iteration to allow absent content or parts, while
validating present parts as an array containing only non-null objects. Route
invalid containers and elements through the terminal adapter error channel
rather than allowing iteration or property access to throw, and add regression
coverage for parts: {} and parts: [null].

// Record Gemini thought signatures for the next stateless tool-result turn. Vertex and
// Antigravity use separate model namespaces so opaque provider state cannot cross routes.
const replayModel = provider.googleMode === "cloud-code-assist" ? antigravityModel : vertexReplayModel;
Expand Down
52 changes: 44 additions & 8 deletions src/adapters/openai-chat.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,18 @@ function invalidChoicesEvent(usage?: OcxUsage): Extract<AdapterEvent, { type: "e
};
}

function invalidToolCallsEvent(usage?: OcxUsage): Extract<AdapterEvent, { type: "error" }> {
return {
type: "error",
message: "upstream response contained invalid tool calls",
...(usage !== undefined ? { usage } : {}),
};
}

function isRecord(value: unknown): value is Record<string, unknown> {
return value !== null && typeof value === "object" && !Array.isArray(value);
}

function developerSystemText(message: OcxMessage): string | undefined {
if (message.role !== "developer") return undefined;
if (typeof message.content === "string") return message.content;
Expand Down Expand Up @@ -916,9 +928,23 @@ export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAd
yield { type: "text_delta", text: delta.content };
}

const toolCalls = delta.tool_calls as { index?: number; id?: string; function?: { name?: string; arguments?: string } }[] | undefined;
if (toolCalls) {
for (const tc of toolCalls) {
const rawToolCalls = delta.tool_calls;
if (rawToolCalls !== undefined) {
// A claimed tool-call payload is not benign padding. Dropping it can leave the
// matching result permanently orphaned, so malformed nested shapes fail closed
// through the adapter error channel instead of escaping as TypeError (#1325).
if (!Array.isArray(rawToolCalls)) {
return yield* terminateWithError(invalidToolCallsEvent(pendingUsage));
}
for (const rawToolCall of rawToolCalls) {
if (!isRecord(rawToolCall)) {
return yield* terminateWithError(invalidToolCallsEvent(pendingUsage));
}
const tc = rawToolCall as {
index?: number;
id?: string;
function?: { name?: string; arguments?: string };
};
Comment on lines +931 to +947

@coderabbitai coderabbitai Bot Aug 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Validate nested streaming tool-call fields and final call state.

Lines 943-947 only validate that each entry is an object. A fragment such as { "function": [] } or { "id": 7 } is accepted. The adapter can then emit a tool_call_start with an empty or non-string id or name, followed by done.

Accept partial fragments when fields are absent. If a field is present, require its expected type. Before flushToolCalls emits events, reject calls that never received a string id, name, and arguments value. Add streaming regressions for malformed function, id, name, and arguments.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/adapters/openai-chat.ts` around lines 931 - 947, Strengthen validation in
the streaming tool-call handling around rawToolCall and flushToolCalls: accept
absent partial fields, but reject present id and
function.name/function.arguments values unless they are strings, and reject
function unless it is an object when provided. Before emitting final tool-call
events, require each call to have received string id, name, and arguments
values; route all invalid cases through
terminateWithError(invalidToolCallsEvent(pendingUsage)). Add regressions
covering malformed function, id, name, and arguments payloads.

const key = typeof tc.index === "number"
? `i:${tc.index}`
: tc.id
Expand Down Expand Up @@ -1073,11 +1099,21 @@ export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAd
const reasoningText = reasoningTextFrom(msg);
if (reasoningText !== undefined) events.push({ type: "reasoning_raw_delta", text: reasoningText });
if (typeof msg.content === "string") events.push({ type: "text_delta", text: msg.content });
const toolCalls = msg.tool_calls as { id: string; function: { name: string; arguments: string } }[] | undefined;
if (toolCalls) {
for (const tc of toolCalls) {
events.push({ type: "tool_call_start", id: tc.id, name: tc.function.name });
events.push({ type: "tool_call_delta", arguments: tc.function.arguments });
const rawToolCalls = msg.tool_calls;
if (rawToolCalls !== undefined) {
if (!Array.isArray(rawToolCalls)) return [invalidToolCallsEvent(usage)];
for (const rawToolCall of rawToolCalls) {
if (!isRecord(rawToolCall) || !isRecord(rawToolCall.function)) {
return [invalidToolCallsEvent(usage)];
}
const id = rawToolCall.id;
const name = rawToolCall.function.name;
const args = rawToolCall.function.arguments;
if (typeof id !== "string" || typeof name !== "string" || typeof args !== "string") {
return [invalidToolCallsEvent(usage)];
}
events.push({ type: "tool_call_start", id, name });
events.push({ type: "tool_call_delta", arguments: args });
events.push({ type: "tool_call_end" });
}
}
Expand Down
12 changes: 12 additions & 0 deletions tests/google-hardening.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,18 @@ describe("google provider hardening", () => {
expect(events.some(event => event.type === "done")).toBe(false);
});

test("a malformed nested candidate is a terminal stream error", async () => {
const events = await collect(createGoogleAdapter(provider()).parseStream(
sseResponse([{ candidates: [null] }, { candidates: [{ finishReason: "STOP" }] }]),
));

expect(events).toEqual([{
type: "error",
message: "google response contained invalid candidates",
}]);
expect(events.some(event => event.type === "done")).toBe(false);
});

test("EOF residual data frame without a trailing newline is parsed", async () => {
const events = await collect(createGoogleAdapter(provider()).parseStream(
new Response('data:{"candidates":[{"content":{"parts":[{"text":"final"}]},"finishReason":"STOP"}]}', {
Expand Down
39 changes: 39 additions & 0 deletions tests/openai-chat-hardening.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,25 @@ describe("openai-chat non-stream response hardening", () => {

expect(events).toEqual([{ type: "error", message: "upstream response contained invalid choices" }]);
});

test("rejects malformed nested tool calls without throwing", async () => {
const adapter = createOpenAIChatAdapter(provider());
for (const toolCalls of [
{ unexpected: true },
[null],
[{ id: "call_missing_function" }],
]) {
const events = await adapter.parseResponse!(new Response(JSON.stringify({
choices: [{ message: { role: "assistant", tool_calls: toolCalls } }],
usage: { prompt_tokens: 7, completion_tokens: 2 },
})));
expect(events).toEqual([{
type: "error",
message: "upstream response contained invalid tool calls",
usage: { inputTokens: 7, outputTokens: 2 },
}]);
}
});
});

describe("openai-chat stream response hardening", () => {
Expand Down Expand Up @@ -160,6 +179,26 @@ describe("openai-chat stream response hardening", () => {
expect(events.at(-1)).toEqual({ type: "error", message: "malformed upstream SSE data frame" });
expect(events.some(event => event.type === "done")).toBe(false);
});

test("malformed nested streaming tool calls are terminal errors", async () => {
const adapter = createOpenAIChatAdapter(provider());
for (const toolCalls of [{ unexpected: true }, [null]]) {
const response = new Response([
`data: ${JSON.stringify({
choices: [{ delta: { tool_calls: toolCalls } }],
usage: { prompt_tokens: 7, completion_tokens: 2 },
})}\n\n`,
"data: [DONE]\n\n",
].join(""));

const events = await collect(adapter.parseStream(response));
expect(events).toEqual([{
type: "error",
message: "upstream response contained invalid tool calls",
usage: { inputTokens: 7, outputTokens: 2 },
}]);
}
});
});

describe("openai-chat credential hardening", () => {
Expand Down
Loading