diff --git a/lib/structured-output.mjs b/lib/structured-output.mjs index dc4fd2a..a444bd9 100644 --- a/lib/structured-output.mjs +++ b/lib/structured-output.mjs @@ -209,6 +209,22 @@ export function validateJsonSchema(value, schema, path = "$", strict = false, ro return errors; } +// Crash-safe façade over validateJsonSchema (issue #181). The validator recurses on the DATA's +// nesting depth (properties/items/additionalProperties), which the REF_DEPTH_CAP does NOT bound — +// only the ref-chain is. A model reply nested ~2000 levels deep therefore overflowed the stack with +// a RangeError, which the request handler caught as a generic HTTP 500 instead of the spec-correct +// `refusal`. This wrapper converts ANY throw (the deep-data RangeError, or any future recursion +// hazard) into a single validation error, so the structured-output retry loop treats a pathological +// reply as "did not validate" → refusal — never a 500, never a crash. A well-formed reply is +// unaffected: the inner validator returns and this just passes its errors through. +export function validateJsonSchemaSafe(value, schema, path = "$", strict = false, root = schema) { + try { + return validateJsonSchema(value, schema, path, strict, root); + } catch (e) { + return [`${path}: schema validation aborted (${e instanceof RangeError ? "value nesting too deep" : "internal error"})`]; + } +} + function tryJsonParse(s) { try { return { ok: true, value: JSON.parse(s) }; } catch { return { ok: false }; } } diff --git a/server.mjs b/server.mjs index 99354be..2ebdfa9 100644 --- a/server.mjs +++ b/server.mjs @@ -42,7 +42,7 @@ import { dirname, join } from "node:path"; import { homedir } from "node:os"; import { validateKey, recordUsage, getUsageByKey, getUsageTimeline, getRecentUsage, createKey, listKeys, revokeKey, closeDb, checkQuota, updateKeyQuota, getKeyQuota, findKey, cacheHash, getCachedResponse, setCachedResponse, clearCache, getCacheStats, hasCacheControl, singleflight, getInflightStats } from "./keys.mjs"; import { DEFAULT_PORT } from "./lib/constants.mjs"; -import { StructuredOutputError, detectStructuredOutput, validateJsonSchema, extractJsonPayload, structuredSystemInstruction, resolveMaxAttempts } from "./lib/structured-output.mjs"; +import { StructuredOutputError, detectStructuredOutput, validateJsonSchema, validateJsonSchemaSafe, extractJsonPayload, structuredSystemInstruction, resolveMaxAttempts } from "./lib/structured-output.mjs"; import { isLoopbackBind } from "./lib/net.mjs"; import { runTuiTurn, reapStaleTuiSessions, resolveTuiHome, bootTuiPane, tuiPaneHealthy, poolPaneName, POOL_BOOT_MS } from "./lib/tui/session.mjs"; import { detectTuiUpstreamError } from "./lib/tui/transcript.mjs"; @@ -2709,7 +2709,10 @@ async function runStructuredCompletion(upstreamCall, model, messages, conversati continue; } if (structured.mode === "schema" && structured.schema) { - const errs = validateJsonSchema(extracted.value, structured.schema, "$", structured.strict); + // validateJsonSchemaSafe (#181): a pathologically deep model reply overflows the value-depth + // recursion; the safe façade turns that into a validation miss (→ retry → refusal) instead of + // a caught RangeError surfacing as a generic 500. + const errs = validateJsonSchemaSafe(extracted.value, structured.schema, "$", structured.strict); if (errs.length) { lastErr = "schema validation failed: " + errs.slice(0, 5).join("; "); logEvent("warn", "structured_retry", { attempt, reason: "schema", errors: errs.slice(0, 5) }); diff --git a/test-features.mjs b/test-features.mjs index 3c20e21..35f82cf 100644 --- a/test-features.mjs +++ b/test-features.mjs @@ -4331,7 +4331,7 @@ test("stream: /health block is additive and exposes the divergence counter", () }); // ── OpenAI Structured Outputs (response_format) — lib/structured-output.mjs ── -import { detectStructuredOutput, validateJsonSchema, extractJsonPayload, structuredSystemInstruction, StructuredOutputError, resolveMaxAttempts } from "./lib/structured-output.mjs"; +import { detectStructuredOutput, validateJsonSchema, validateJsonSchemaSafe, extractJsonPayload, structuredSystemInstruction, StructuredOutputError, resolveMaxAttempts } from "./lib/structured-output.mjs"; test("detectStructuredOutput: json_schema shape", () => { const d = detectStructuredOutput({ response_format: { type: "json_schema", json_schema: { name: "x", strict: true, schema: { type: "object" } } } }); @@ -4358,6 +4358,28 @@ test("cacheHash: structured marker isolates JSON requests from the conversationa assert.equal(plain, cacheHash("m", msgs, { keyId: "k" })); // unchanged for normal requests }); +// ── validateJsonSchemaSafe (#181): deep value must NOT crash the handler ───── +// A recursive schema + a model reply nested ~thousands deep overflows the value- +// depth recursion → RangeError → the handler used to surface a generic 500. The +// safe façade turns it into a validation miss (→ retry → refusal). Mutation-proof: +// replace the wrapper body with a bare `validateJsonSchema(...)` call and the deep +// test throws instead of returning errors. +test("validateJsonSchemaSafe: pathologically deep value → errors, never throws", () => { + const schema = { $defs: { node: { type: "object", properties: { child: { $ref: "#/$defs/node" } } } }, $ref: "#/$defs/node" }; + let deep = {}; + let cur = deep; + for (let i = 0; i < 6000; i++) { cur.child = {}; cur = cur.child; } // way past any stack limit + let out; + assert.doesNotThrow(() => { out = validateJsonSchemaSafe(deep, schema, "$", true); }, "must not throw a RangeError out to the handler"); + assert.ok(Array.isArray(out) && out.length > 0, "returns a non-empty validation error, so the retry loop yields a refusal not a 500"); +}); + +test("validateJsonSchemaSafe: well-formed value passes through unchanged (byte-identical to the raw validator)", () => { + const schema = { type: "object", required: ["name", "age"], properties: { name: { type: "string" }, age: { type: "integer" } } }; + assert.deepEqual(validateJsonSchemaSafe({ name: "a", age: 3 }, schema), validateJsonSchema({ name: "a", age: 3 }, schema)); + assert.deepEqual(validateJsonSchemaSafe({ name: "a" }, schema), validateJsonSchema({ name: "a" }, schema)); // error case matches too +}); + test("validateJsonSchema: valid object passes", () => { assert.deepEqual(validateJsonSchema({ name: "a", age: 3 }, { type: "object", required: ["name", "age"], properties: { name: { type: "string" }, age: { type: "integer" } } }), []); });