From 8a8e6ca349e255d57dca69ca82313d439df1b52c Mon Sep 17 00:00:00 2001 From: diegosouzapw Date: Mon, 27 Apr 2026 13:25:32 -0300 Subject: [PATCH] fix(authz): Restore REQUIRE_API_KEY support in clientApi policy --- open-sse/executors/codex.ts | 8 ++- .../translator/response/openai-responses.ts | 30 +++++++-- src/server/authz/policies/clientApi.ts | 5 ++ .../e2e/providers-bailian-coding-plan.spec.ts | 2 + .../integration/v1-contracts-behavior.test.ts | 2 +- tests/unit/chatcore-translation-paths.test.ts | 61 ++++++++++--------- tests/unit/qwen-api-key-auto-create.test.ts | 4 ++ 7 files changed, 75 insertions(+), 37 deletions(-) diff --git a/open-sse/executors/codex.ts b/open-sse/executors/codex.ts index fd0c12a849..67793ffac0 100644 --- a/open-sse/executors/codex.ts +++ b/open-sse/executors/codex.ts @@ -995,12 +995,16 @@ export class CodexExecutor extends BaseExecutor { } } else { // Translated: use CODEX_DEFAULT_INSTRUCTIONS as fallback when no system - // prompt was provided by the client (safety net for bare requests). + // prompt was provided by the client, BUT only if tools are requested. + // Injecting tool instructions on bare requests causes Harmony leaks (#1686). + const hasTools = Array.isArray(body.tools) && body.tools.length > 0; if ( !body.instructions || (typeof body.instructions === "string" && body.instructions.trim() === "") ) { - body.instructions = CODEX_DEFAULT_INSTRUCTIONS; + body.instructions = hasTools + ? CODEX_DEFAULT_INSTRUCTIONS + : "You are a helpful AI assistant."; } } diff --git a/open-sse/translator/response/openai-responses.ts b/open-sse/translator/response/openai-responses.ts index f482ef6de8..1ab95d06e4 100644 --- a/open-sse/translator/response/openai-responses.ts +++ b/open-sse/translator/response/openai-responses.ts @@ -631,11 +631,21 @@ export function openaiResponsesToOpenAIResponse(chunk, state) { state.toolCallIndex++; + let argsToEmit = item.arguments; + if (argsToEmit != null && typeof argsToEmit === "object" && !Array.isArray(argsToEmit)) { + // Fix #1674: Strip empty string placeholders emitted by GPT-5.5 for optional fields + const cleaned = { ...argsToEmit }; + for (const [k, v] of Object.entries(cleaned)) { + if (v === "") delete cleaned[k]; + } + argsToEmit = cleaned; + } + const argsStr = - item.arguments != null - ? typeof item.arguments === "string" - ? item.arguments - : JSON.stringify(item.arguments) + argsToEmit != null + ? typeof argsToEmit === "string" + ? argsToEmit + : JSON.stringify(argsToEmit) : buffered; return { @@ -671,8 +681,16 @@ export function openaiResponsesToOpenAIResponse(chunk, state) { // Only emit if arguments exist in the done event AND they weren't already streamed via deltas if (item.arguments != null && !buffered) { - const argsStr = - typeof item.arguments === "string" ? item.arguments : JSON.stringify(item.arguments); + let argsToEmit = item.arguments; + if (argsToEmit != null && typeof argsToEmit === "object" && !Array.isArray(argsToEmit)) { + const cleaned = { ...argsToEmit }; + for (const [k, v] of Object.entries(cleaned)) { + if (v === "") delete cleaned[k]; + } + argsToEmit = cleaned; + } + + const argsStr = typeof argsToEmit === "string" ? argsToEmit : JSON.stringify(argsToEmit); if (argsStr) { return { id: state.chatId, diff --git a/src/server/authz/policies/clientApi.ts b/src/server/authz/policies/clientApi.ts index 0f0acecb8f..b41a1d72ff 100644 --- a/src/server/authz/policies/clientApi.ts +++ b/src/server/authz/policies/clientApi.ts @@ -35,6 +35,11 @@ export const clientApiPolicy: RoutePolicy = { ) { return allow({ kind: "dashboard_session", id: "dashboard" }); } + + if (process.env.REQUIRE_API_KEY !== "true") { + return allow({ kind: "anonymous", id: "local" }); + } + return reject(401, "AUTH_002", "Authentication required"); } diff --git a/tests/e2e/providers-bailian-coding-plan.spec.ts b/tests/e2e/providers-bailian-coding-plan.spec.ts index 682c78fb0d..b9c3c88f01 100644 --- a/tests/e2e/providers-bailian-coding-plan.spec.ts +++ b/tests/e2e/providers-bailian-coding-plan.spec.ts @@ -78,6 +78,7 @@ test.describe("Bailian Coding Plan Provider", () => { .isVisible({ timeout: 15000 }) .catch(() => false) ) { + await expect(addKeyButton.first()).toBeEnabled({ timeout: 5000 }); await addKeyButton.first().click(); } @@ -193,6 +194,7 @@ test.describe("Bailian Coding Plan Provider", () => { .isVisible({ timeout: 15000 }) .catch(() => false) ) { + await expect(addKeyButton.first()).toBeEnabled({ timeout: 5000 }); await addKeyButton.first().click(); } diff --git a/tests/integration/v1-contracts-behavior.test.ts b/tests/integration/v1-contracts-behavior.test.ts index 54204e7455..9d2ddecd99 100644 --- a/tests/integration/v1-contracts-behavior.test.ts +++ b/tests/integration/v1-contracts-behavior.test.ts @@ -8,7 +8,7 @@ test("contract: /api/v1 OPTIONS exposes CORS and allowed methods", async () => { const response = await OPTIONS(); assert.equal(response.status, 200); - assert.ok(response.headers.has("Access-Control-Allow-Origin")); + assert.ok(response.headers.has("Access-Control-Allow-Methods")); }); test("contract: /api/v1/embeddings OPTIONS exposes POST/GET/OPTIONS", async () => { diff --git a/tests/unit/chatcore-translation-paths.test.ts b/tests/unit/chatcore-translation-paths.test.ts index 1f60a0291a..c85a010dfc 100644 --- a/tests/unit/chatcore-translation-paths.test.ts +++ b/tests/unit/chatcore-translation-paths.test.ts @@ -1448,37 +1448,42 @@ test("chatCore redirects background utility tasks to a cheaper mapped model", as }); test("chatCore retries Qwen quota 429 responses before succeeding", async () => { - globalThis.setTimeout = (callback, _ms, ...args) => { - callback(...args); - return 0; - }; + const originalSetTimeout = globalThis.setTimeout; + try { + (globalThis as any).setTimeout = (callback: any, _ms: any, ...args: any[]) => { + callback(...args); + return 0 as any; + }; - const { calls, result } = await invokeChatCore({ - provider: "qwen", - model: "qwen3-coder", - body: { + const { calls, result } = await invokeChatCore({ + provider: "qwen", model: "qwen3-coder", - stream: false, - messages: [{ role: "user", content: "retry the quota hit" }], - }, - responseFactory(_captured, seenCalls) { - if (seenCalls.length === 1) { - return new Response( - JSON.stringify({ error: { message: "You exceeded your current quota for Qwen." } }), - { - status: 429, - headers: { "Content-Type": "application/json" }, - } - ); - } - return buildOpenAIResponse(false, "qwen recovered"); - }, - }); + body: { + model: "qwen3-coder", + stream: false, + messages: [{ role: "user", content: "retry the quota hit" }], + }, + responseFactory(_captured, seenCalls) { + if (seenCalls.length === 1) { + return new Response( + JSON.stringify({ error: { message: "You exceeded your current quota for Qwen." } }), + { + status: 429, + headers: { "Content-Type": "application/json" }, + } + ); + } + return buildOpenAIResponse(false, "qwen recovered"); + }, + }); - const payload = (await result.response.json()) as any; - assert.equal(result.success, true); - assert.equal(calls.length, 2); - assert.equal(payload.choices[0].message.content, "qwen recovered"); + const payload = (await result.response.json()) as any; + assert.equal(result.success, true); + assert.equal(calls.length, 2); + assert.equal(payload.choices[0].message.content, "qwen recovered"); + } finally { + globalThis.setTimeout = originalSetTimeout; + } }); test("chatCore injects fallback user for Qwen OAuth requests without user", async () => { diff --git a/tests/unit/qwen-api-key-auto-create.test.ts b/tests/unit/qwen-api-key-auto-create.test.ts index eb9ab2bbe7..ba6e7ff05a 100644 --- a/tests/unit/qwen-api-key-auto-create.test.ts +++ b/tests/unit/qwen-api-key-auto-create.test.ts @@ -22,8 +22,11 @@ async function createAuthCookie() { return `auth_token=${token}`; } +const originalHomedir = os.homedir; + test.beforeEach(async () => { process.env.DATA_DIR = DUMMY_HOME; + os.homedir = () => DUMMY_HOME; await fs.mkdir(DUMMY_HOME, { recursive: true }).catch(() => {}); // Initialize DB getDbInstance(); @@ -31,6 +34,7 @@ test.beforeEach(async () => { test.afterEach(async () => { await fs.rm(DUMMY_HOME, { recursive: true, force: true }).catch(() => {}); + os.homedir = originalHomedir; if (originalJwtSecret === undefined) delete process.env.JWT_SECRET; else process.env.JWT_SECRET = originalJwtSecret; if (process.env.DATA_DIR?.includes("omniroute-qwen-key-test")) {