fix(translator): strip empty cloud_base_branch from Cursor Subagent tool call (port from 9router#2446)

The Responses->Chat tool-arg cleanup (stripEmptyOptionalToolArgs) only stripped
empty-string/empty-array optional args for Claude Code's Read tool. Cursor's local
Subagent tool call therefore passed through with the cloud-only field
cloud_base_branch: "", which Cursor rejects ("cloud_base_branch may only be specified
when environment equals cloud") before starting the subagent. Extend the cleanup to an
allowlist of Read + Subagent; arbitrary tools stay untouched.

Reported-by: like3213934360-lab (https://github.com/decolua/9router/issues/2446)
This commit is contained in:
Diego Rodrigues de Sa e Souza 2026-07-09 05:09:00 -03:00
parent f4fb0d310b
commit 617828f1c3
3 changed files with 67 additions and 2 deletions

View file

@ -14,6 +14,7 @@ _Living section — bullets land here as PRs merge into `release/v3.8.47` (paral
### 🐛 Bug Fixes
- **fix(translator):** Cursor's local **Subagent** tool call is no longer rejected with `cloud_base_branch may only be specified when environment equals cloud` — the Responses→Chat tool-arg cleanup (`stripEmptyOptionalToolArgs`) was scoped to Claude Code's `Read` tool only, so Cursor's `Subagent` tool passed through with the cloud-only `cloud_base_branch: ""` (Cursor treats an empty string as "specified" and rejects the call before starting the local subagent). The cleanup now covers an allowlist of `Read` + `Subagent`; arbitrary tools are still left untouched (empty strings/arrays can be valid payloads for them). Regression guard: `tests/unit/openai-responses-subagent-strip-2446.test.ts`. (thanks @like3213934360-lab)
- **fix(auth):** an API key restricted via `allowedModels`/`allowedCombos` could bypass that restriction entirely over the Codex Responses-over-WebSocket bridge ([#6564](https://github.com/diegosouzapw/OmniRoute/issues/6564)) — `prepare()` in `src/app/api/internal/codex-responses-ws/route.ts` authenticated the WS bridge's API key (`authenticate()`/`authorizeWebSocketHandshake()`) and honored `allowedConnections`, but never called `enforceApiKeyPolicy()`, the same model/combo policy gate the HTTP `/v1/responses` path enforces via `handleChat()` — so a key scoped to e.g. `combo/model-1.0` could still reach a direct Codex model like `gpt-5.5` through this transport, as long as an eligible Codex OAuth connection existed. The bridge's WS auth token arrives via query params (`api_key`/`token`/`access_token`), not a normal `Authorization` header, so a new `enforceCodexWsApiKeyPolicy()` builds an equivalent `Request` carrying an explicit `Authorization: Bearer <apiKey>` header and calls `enforceApiKeyPolicy()` against the CLIENT-requested model, before any Codex-specific model remapping or credential selection. Regression guard: `tests/unit/codex-ws-policy-enforcement-6564.test.ts` (a model-restricted key is rejected 403 before reaching credential selection; a combo-restricted key is rejected 403 requesting a disallowed combo; a key that DOES allow the requested model still proceeds past policy).
- **fix(security):** loopback-gate `/api/middleware/*` so a leaked JWT over a tunnel can't install or trigger a middleware hook — middleware hooks compile + run arbitrary JS via `new vm.Script` on the request hot path (`src/lib/middleware/registry.ts`), the same RCE class as the already-gated `/api/plugins/*`; `/api/middleware/` is now in `LOCAL_ONLY_API_PREFIXES` so loopback enforcement runs unconditionally before any auth check (Hard Rules #15 + #17). Regression guard: `tests/unit/route-guard-middleware-local-only.test.ts`. ([#6541](https://github.com/diegosouzapw/OmniRoute/pull/6541)) — see PR. (thanks @developerjillur)
- **fix(startup):** AgentBridge's MITM server no longer fails to start with `ROUTER_API_KEY is required` on a normal install ([#6403](https://github.com/diegosouzapw/OmniRoute/issues/6403)) — `POST /api/tools/agent-bridge/server` resolved the spawned MITM child's router key from only an explicit `apiKey` body field (never sent by the AgentBridge UI — the schema has no such field) and the `ROUTER_API_KEY` env var (unset by default), so `startMitm()` always received `""` and the child hard-exited, even though OmniRoute already had a usable API key in its own DB. A new `resolveRouterApiKey()` now falls back to `pickApiKeyForInternalUse()` (the same DB-backed selector the combo-health-check / cloud-sync internal probes use), resolving in order: explicit key → `ROUTER_API_KEY` env → an existing DB key. Regression guard: `tests/unit/agentbridge-mitm-router-key-6403.test.ts`.

View file

@ -5,13 +5,20 @@ export function normalizeToolName(value) {
return typeof value === "string" ? value.trim() : "";
}
// Tools whose empty-string/empty-array optional args are safe to strip. Arbitrary
// tools are left untouched because an empty string/array can be a valid payload.
// - "Read": Claude Code's Read tool (empty `pages`) — #2937.
// - "Subagent": Cursor's local subagent tool emits a cloud-only `cloud_base_branch: ""`,
// which Cursor rejects unless environment is cloud — ported from decolua/9router#2446.
const STRIPPABLE_EMPTY_ARG_TOOLS = new Set(["Read", "Subagent"]);
export function stripEmptyOptionalToolArgs(value, toolName) {
if (value == null) return value;
if (typeof value === "string") {
// JSON-string cleanup is intentionally scoped to Claude Code's Read tool.
// JSON-string cleanup is intentionally scoped to the allowlisted tools above.
// For arbitrary tools, empty strings/arrays may be valid user payloads.
if (toolName !== "Read") return value;
if (!STRIPPABLE_EMPTY_ARG_TOOLS.has(toolName)) return value;
try {
const parsed = JSON.parse(value);
if (Array.isArray(parsed) || typeof parsed !== "object" || parsed === null) return value;

View file

@ -0,0 +1,57 @@
import { test } from "node:test";
import assert from "node:assert/strict";
// Regression guard for the Cursor `Subagent` tool call carrying the cloud-only
// optional field `cloud_base_branch` as an empty string. Cursor rejects the call
// ("cloud_base_branch may only be specified when environment equals cloud") when a
// local subagent tool call includes the field at all. The Responses->Chat translator
// strips empty-string/empty-array optional fields, but that cleanup was scoped to
// Claude Code's `Read` tool only; it must also cover Cursor's `Subagent` tool.
// Ported from decolua/9router#2446.
const LEAF = "../../open-sse/translator/response/openai-responses/pureHelpers.ts";
test("Subagent tool: strips empty-string cloud_base_branch, keeps populated fields", async () => {
const { stripEmptyOptionalToolArgs } = await import(LEAF);
const raw = JSON.stringify({
description: "subagent connectivity test",
prompt: "hello",
readonly: true,
subagent_type: "generalPurpose",
file_attachments: [],
environment: "local",
cloud_base_branch: "",
interrupt: false,
run_in_background: false,
});
const cleaned = JSON.parse(stripEmptyOptionalToolArgs(raw, "Subagent"));
// The offending empty optional field must be gone.
assert.equal("cloud_base_branch" in cleaned, false);
// Empty array optional also dropped (same rule as Read).
assert.equal("file_attachments" in cleaned, false);
// Populated / meaningful fields preserved — including falsy booleans.
assert.equal(cleaned.description, "subagent connectivity test");
assert.equal(cleaned.prompt, "hello");
assert.equal(cleaned.readonly, true);
assert.equal(cleaned.subagent_type, "generalPurpose");
assert.equal(cleaned.environment, "local");
assert.equal(cleaned.interrupt, false);
assert.equal(cleaned.run_in_background, false);
});
test("Read tool cleanup remains intact (no regression)", async () => {
const { stripEmptyOptionalToolArgs } = await import(LEAF);
const raw = JSON.stringify({ file_path: "/a.ts", pages: "" });
const cleaned = JSON.parse(stripEmptyOptionalToolArgs(raw, "Read"));
assert.equal("pages" in cleaned, false);
assert.equal(cleaned.file_path, "/a.ts");
});
test("arbitrary tools keep empty strings/arrays (unchanged pass-through)", async () => {
const { stripEmptyOptionalToolArgs } = await import(LEAF);
const raw = JSON.stringify({ query: "", tags: [] });
// Not on the allowlist -> returned verbatim.
assert.equal(stripEmptyOptionalToolArgs(raw, "SomeOtherTool"), raw);
});