From b48ec7cc709943a23aa9482473267c1f19a14267 Mon Sep 17 00:00:00 2001
From: dengyie <dengyie@users.noreply.github.com>
Date: Mon, 5 Oct 2026 04:15:24 +0800
Subject: [PATCH] fix: validate request body shape at the single parse point;
 sanitize 502 detail
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Review-reproduced on 2026-10-05: JSON.parse('null') parses legally, but
handleChatCompletions treated a null parsed body as 'error response already
written' and returned without writing anything — the client hung until its own
timeout. Bodies of 123/'str' hit the typeof check inside
stripReasoningTriggers ('in' on a primitive), throwing a TypeError that the
upstream catch block classified as a connect failure: a misleading 502 that
counted toward the errors dashboard and leaked the internal exception text.

Root fix: readJsonBody — the single place every route parses a body — now
rejects non-object JSON (null / number / string / array) with 400, covering
chat completions, messages, and count_tokens with the correct error envelope
per route. The 502 message is fixed to 'upstream connect failed' with details
kept in logs only.

Tests: malformed-body matrix 400 regression (OpenAI + Anthropic envelopes),
upstream connection reset → 502 with the fixed message, and a streaming-path
variant for the reasoning-toggle strip assertions. 67/67 green.
---
 src/proxy.mjs       |  3 ++-
 src/server.mjs      | 11 ++++++++++-
 test/proxy.test.mjs | 45 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/src/proxy.mjs b/src/proxy.mjs
index c6d5d50..5d91f97 100644
--- a/src/proxy.mjs
+++ b/src/proxy.mjs
@@ -50,7 +50,8 @@ export async function forward({ reqId, payload, wantStream, res, transform }) {
       return { handled: false, status: 504, body: { error: { message: 'upstream connect timeout' } } };
     }
     log.error(reqId, 'upstream fetch failed', { err: String(e).slice(0, 200) });
-    return { handled: false, status: 502, body: { error: { message: `upstream connect failed: ${e.cause || e}` } } };
+    // 错误详情只进日志：内部异常串（含 cause/payload 片段）不外泄给客户端
+    return { handled: false, status: 502, body: { error: { message: 'upstream connect failed' } } };
   }
   clearTimeout(timer);
 
diff --git a/src/server.mjs b/src/server.mjs
index c8109e0..9ded0a0 100644
--- a/src/server.mjs
+++ b/src/server.mjs
@@ -63,10 +63,19 @@ async function readJsonBody(req, res, reqId, { envelope = false } = {}) {
     else throw e;
     return null;
   }
-  try { return JSON.parse(raw); } catch {
+  let parsed;
+  try { parsed = JSON.parse(raw); } catch {
     send(res, 400, envelope ? anthropicError(400, { error: { message: 'request body is not valid JSON' } }) : { error: { message: 'request body is not valid JSON', type: 'invalid_request_error' } });
     return null;
   }
+  // 合法 JSON ≠ 合法请求体：null/数字/字符串/数组都是合法 JSON，但流入下游会
+  // 退化（null 体会静默挂起不回包、原始类型触发内部 TypeError 变误导性 502），
+  // 在唯一解析点统一 400，三条路由（chat/messages/count_tokens）一并兜住
+  if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) {
+    send(res, 400, envelope ? anthropicError(400, { error: { message: 'request body must be a JSON object' } }) : { error: { message: 'request body must be a JSON object', type: 'invalid_request_error' } });
+    return null;
+  }
+  return parsed;
 }
 
 async function handleChatCompletions(req, res, reqId) {
diff --git a/test/proxy.test.mjs b/test/proxy.test.mjs
index 34bb95b..9e60935 100644
--- a/test/proxy.test.mjs
+++ b/test/proxy.test.mjs
@@ -32,6 +32,11 @@ const mockServer = http.createServer((req, res) => {
       // 连接后不回任何字节 → 代理应在上游连接超时后回 504（而不是挂死客户端）
       return;
     }
+    if (parsed.model === 'mock-reset') {
+      // 收到请求后直接断连 → 代理应回 502，且错误消息不泄漏内部异常细节
+      res.socket.destroy();
+      return;
+    }
     if (parsed.model === 'mock-stall') {
       // 出响应头后停滞 → 代理流空闲看门狗应 abort 并结束响应
       res.writeHead(200, { 'Content-Type': 'text/event-stream' });
@@ -184,6 +189,32 @@ test('invalid json → 400', async () => {
   assert.equal(res.status, 400);
 });
 
+test('合法 JSON 但非对象 → 400 可解析（回归：null 体曾静默挂死、原始类型曾 TypeError 劣化 502）', async () => {
+  // 两个历史缺陷同一根因（入口无形状校验）：
+  // ① body=null 曾直接 return 不回包 → 客户端挂死到自身超时；
+  // ② body=123/"str" 曾在 stripReasoningTriggers 的 in 操作符上抛 TypeError → 误导性 502 计入 errors 看板
+  for (const body of ['null', '123', '"str"', '[1,2]']) {
+    const res = await fetch(`http://127.0.0.1:${PROXY_PORT}/v1/chat/completions`, {
+      method: 'POST',
+      headers: { Authorization: `Bearer ${KEY}`, 'Content-Type': 'application/json' },
+      body,
+    });
+    assert.equal(res.status, 400, `body=${body} must be 400`);
+    const j = await res.json(); // 可解析 = 响应确实写出了（挂死回归即在此断言失败）
+    assert.equal(j.error.type, 'invalid_request_error');
+  }
+  // /v1/messages 同样兜住，且包 Anthropic envelope
+  const m = await fetch(`http://127.0.0.1:${PROXY_PORT}/v1/messages`, {
+    method: 'POST',
+    headers: { 'x-api-key': KEY, 'Content-Type': 'application/json' },
+    body: 'null',
+  });
+  assert.equal(m.status, 400);
+  const mj = await m.json();
+  assert.equal(mj.type, 'error');
+  assert.equal(mj.error.type, 'invalid_request_error');
+});
+
 test('body over 4MiB → 413', async () => {
   const big = JSON.stringify({ messages: [{ role: 'user', content: 'a'.repeat(5 * 1024 * 1024) }] });
   const res = await fetch(`http://127.0.0.1:${PROXY_PORT}/v1/chat/completions`, {
@@ -210,6 +241,14 @@ test('upstream 500 → transparent pass-through', async () => {
   assert.equal(j.error.message, 'mock upstream boom');
 });
 
+test('upstream 断连 → 502 且消息不外泄内部异常细节', async () => {
+  // mock-reset 直接触发 fetch 异常（非超时路径）：错误详情只进日志，客户端消息固定
+  const r = await chat({ model: 'mock-reset', messages: [] });
+  assert.equal(r.status, 502);
+  const j = await r.json();
+  assert.equal(j.error.message, 'upstream connect failed');
+});
+
 test('client abort cancels upstream (stop burning tokens)', async () => {
   const ac = new AbortController();
   const r = await fetch(`http://127.0.0.1:${PROXY_PORT}/v1/chat/completions`, {
@@ -329,4 +368,10 @@ test('reasoning_effort 出站剥除（真机教训：触发上游 thinking 变
   await chat({ model: 'mock-model', messages: [], stream: false, enable_thinking: true, thinking: { type: 'enabled' } });
   assert.equal(upstreamState.lastBody.enable_thinking, undefined);
   assert.equal(upstreamState.lastBody.thinking, undefined);
+
+  // 流式路径同一剥除（真实 CPA 渠道链走的是流式）：共享同一 fetch 出站点，
+  // 断言钉住流式分支防未来重构把剥除挪进分支
+  await chat({ model: 'mock-model', messages: [], stream: true, reasoning_effort: 'high' });
+  assert.equal(upstreamState.lastBody.reasoning_effort, undefined, 'streaming path must strip too');
+  assert.equal(upstreamState.lastBody.stream, true);
 });