diff --git a/changelog.d/fixes/13690-gemini-schema-properties-name-collision.md b/changelog.d/fixes/13690-gemini-schema-properties-name-collision.md new file mode 100644 index 0000000000..949662a4a8 --- /dev/null +++ b/changelog.d/fixes/13690-gemini-schema-properties-name-collision.md @@ -0,0 +1 @@ +- **fix(translator):** prevent schema property name collisions (e.g. `properties`, `required`) in Gemini schema sanitizer ([#13690](https://github.com/diegosouzapw/OmniRoute/pull/13690)) — thanks @zcrew0x diff --git a/open-sse/translator/helpers/geminiHelper.ts b/open-sse/translator/helpers/geminiHelper.ts index f0c2fa79dc..7571c378eb 100644 --- a/open-sse/translator/helpers/geminiHelper.ts +++ b/open-sse/translator/helpers/geminiHelper.ts @@ -454,6 +454,20 @@ function decodeJsonPointerSegment(segment: unknown): string { return String(segment).replace(/~1/g, "/").replace(/~0/g, "~"); } +// Helper: Recurse into schema children without treating the properties map itself as a SchemaNode. +function forEachSubschema(record: JsonRecord, visitor: (sub: unknown) => void): void { + for (const [key, value] of Object.entries(record)) { + if (!value || typeof value !== "object") continue; + if (SCHEMA_MAP_KEYS.has(key) && !Array.isArray(value)) { + for (const subSchema of Object.values(value as JsonRecord)) { + visitor(subSchema); + } + } else { + visitor(value); + } + } +} + function resolveLocalReference(root: unknown, ref: unknown): unknown | null { if (typeof ref !== "string" || !ref.startsWith("#/")) return null; @@ -538,21 +552,7 @@ function removeUnsupportedKeywords(obj: unknown, keywords: Set): void { delete record[key]; } } - // Recurse into remaining values. `properties` is a map keyed by arbitrary, - // user-defined property NAMES — a tool may legitimately declare a property - // called `pattern`, `enum`, `minLength`, etc. Descend into each property's - // subschema, but never run keyword-deletion against the property names - // themselves, or glob/grep-style tools lose their `pattern` argument (#1368). - for (const [key, value] of Object.entries(record)) { - if (!value || typeof value !== "object") continue; - if (key === "properties" && !Array.isArray(value)) { - for (const subSchema of Object.values(value as JsonRecord)) { - removeUnsupportedKeywords(subSchema, keywords); - } - } else { - removeUnsupportedKeywords(value, keywords); - } - } + forEachSubschema(record, (sub) => removeUnsupportedKeywords(sub, keywords)); } function normalizeAdditionalProperties(obj: unknown): void { @@ -574,28 +574,27 @@ function normalizeAdditionalProperties(obj: unknown): void { delete record.additionalProperties; } - for (const value of Object.values(record)) { - if (value && typeof value === "object") { - normalizeAdditionalProperties(value); - } - } + forEachSubschema(record, normalizeAdditionalProperties); } // Convert const to enum function convertConstToEnum(obj: unknown): void { if (!obj || typeof obj !== "object") return; + if (Array.isArray(obj)) { + for (const item of obj) { + convertConstToEnum(item); + } + return; + } + const record = obj as JsonRecord; if (record.const !== undefined && !record.enum) { record.enum = [record.const]; delete record.const; } - for (const value of Object.values(record)) { - if (value && typeof value === "object") { - convertConstToEnum(value); - } - } + forEachSubschema(record, convertConstToEnum); } // Convert enum values to strings (Gemini requires string enum values) @@ -603,6 +602,13 @@ function convertConstToEnum(obj: unknown): void { function convertEnumValuesToStrings(obj: unknown): void { if (!obj || typeof obj !== "object") return; + if (Array.isArray(obj)) { + for (const item of obj) { + convertEnumValuesToStrings(item); + } + return; + } + const record = obj as JsonRecord; if (record.enum && Array.isArray(record.enum)) { // Gemini only supports enum for string types, not integer @@ -616,17 +622,20 @@ function convertEnumValuesToStrings(obj: unknown): void { } } - for (const value of Object.values(record)) { - if (value && typeof value === "object") { - convertEnumValuesToStrings(value); - } - } + forEachSubschema(record, convertEnumValuesToStrings); } // Merge allOf schemas function mergeAllOf(obj: unknown): void { if (!obj || typeof obj !== "object") return; + if (Array.isArray(obj)) { + for (const item of obj) { + mergeAllOf(item); + } + return; + } + const record = obj as JsonRecord; if (record.allOf && Array.isArray(record.allOf)) { const merged: { properties?: JsonRecord; required?: string[] } = {}; @@ -659,11 +668,7 @@ function mergeAllOf(obj: unknown): void { } } - for (const value of Object.values(record)) { - if (value && typeof value === "object") { - mergeAllOf(value); - } - } + forEachSubschema(record, mergeAllOf); } // Select best schema from anyOf/oneOf @@ -697,6 +702,13 @@ function selectBest(items: unknown[]): number { function flattenAnyOfOneOf(obj: unknown): void { if (!obj || typeof obj !== "object") return; + if (Array.isArray(obj)) { + for (const item of obj) { + flattenAnyOfOneOf(item); + } + return; + } + const record = obj as JsonRecord; if (record.anyOf && Array.isArray(record.anyOf) && record.anyOf.length > 0) { const nonNullSchemas = record.anyOf.filter((s) => s && toRecord(s).type !== "null"); @@ -718,28 +730,27 @@ function flattenAnyOfOneOf(obj: unknown): void { } } - for (const value of Object.values(record)) { - if (value && typeof value === "object") { - flattenAnyOfOneOf(value); - } - } + forEachSubschema(record, flattenAnyOfOneOf); } // Flatten type arrays function flattenTypeArrays(obj: unknown): void { if (!obj || typeof obj !== "object") return; + if (Array.isArray(obj)) { + for (const item of obj) { + flattenTypeArrays(item); + } + return; + } + const record = obj as JsonRecord; if (record.type && Array.isArray(record.type)) { const nonNullTypes = record.type.filter((t) => t !== "null"); record.type = nonNullTypes.length > 0 ? nonNullTypes[0] : "string"; } - for (const value of Object.values(record)) { - if (value && typeof value === "object") { - flattenTypeArrays(value); - } - } + forEachSubschema(record, flattenTypeArrays); } // Clean JSON Schema for Antigravity API compatibility - removes unsupported keywords recursively @@ -818,6 +829,13 @@ export function cleanJSONSchemaForAntigravity( function cleanupRequired(obj: unknown): void { if (!obj || typeof obj !== "object") return; + if (Array.isArray(obj)) { + for (const item of obj) { + cleanupRequired(item); + } + return; + } + const record = obj as JsonRecord; if (record.required && Array.isArray(record.required) && record.properties) { const properties = toRecord(record.properties); @@ -832,12 +850,7 @@ export function cleanJSONSchemaForAntigravity( } } - // Recurse into nested objects - for (const value of Object.values(record)) { - if (value && typeof value === "object") { - cleanupRequired(value); - } - } + forEachSubschema(record, cleanupRequired); } cleanupRequired(cleaned); @@ -846,6 +859,13 @@ export function cleanJSONSchemaForAntigravity( function addPlaceholders(obj: unknown): void { if (!obj || typeof obj !== "object") return; + if (Array.isArray(obj)) { + for (const item of obj) { + addPlaceholders(item); + } + return; + } + const record = obj as JsonRecord; if (record.type === "object") { if (!record.properties || Object.keys(toRecord(record.properties)).length === 0) { @@ -859,12 +879,7 @@ export function cleanJSONSchemaForAntigravity( } } - // Recurse into nested objects - for (const value of Object.values(record)) { - if (value && typeof value === "object") { - addPlaceholders(value); - } - } + forEachSubschema(record, addPlaceholders); } addPlaceholders(cleaned); @@ -889,12 +904,7 @@ export function cleanJSONSchemaForAntigravity( record.type = "object"; } - // Recurse into remaining values. - for (const value of Object.values(record)) { - if (value && typeof value === "object") { - injectObjectType(value); - } - } + forEachSubschema(record, injectObjectType); } injectObjectType(cleaned); @@ -917,12 +927,7 @@ export function cleanJSONSchemaForAntigravity( record.items = { type: "string" }; } - // Recurse into remaining values. - for (const value of Object.values(record)) { - if (value && typeof value === "object") { - ensureArrayItems(value); - } - } + forEachSubschema(record, ensureArrayItems); } ensureArrayItems(cleaned); diff --git a/tests/unit/gemini-schema-properties-name-collision-13057.test.ts b/tests/unit/gemini-schema-properties-name-collision-13057.test.ts new file mode 100644 index 0000000000..311816ae12 --- /dev/null +++ b/tests/unit/gemini-schema-properties-name-collision-13057.test.ts @@ -0,0 +1,54 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +const { cleanJSONSchemaForAntigravity } = + await import("../../open-sse/translator/helpers/geminiHelper.ts"); + +test('#13057 should not inject type: "object" into properties map when a property is named "properties"', () => { + const inputSchema = { + type: "object", + properties: { + action: { type: "string" }, + properties: { + type: "array", + items: { type: "string" }, + }, + }, + required: ["action"], + }; + + const cleaned = cleanJSONSchemaForAntigravity(inputSchema) as Record; + const cleanedProperties = cleaned.properties as Record>; + + assert.equal(cleaned.type, "object"); + assert.ok(cleanedProperties); + assert.ok(cleanedProperties.action); + assert.ok(cleanedProperties.properties); + // The properties map itself must NOT have a "type" property injected + assert.equal(cleanedProperties.type, undefined); + assert.equal(cleanedProperties.properties.type, "array"); +}); + +test('#13477 should not inject type: "object" into properties map when a property is named "required" (delivery.pin case)', () => { + const inputSchema = { + type: "object", + properties: { + enabled: { type: "boolean" }, + notify: { type: "boolean" }, + required: { type: "boolean" }, + }, + required: ["enabled"], + }; + + const cleaned = cleanJSONSchemaForAntigravity(inputSchema) as Record; + const cleanedProperties = cleaned.properties as Record>; + + assert.equal(cleaned.type, "object"); + assert.ok(cleanedProperties); + assert.ok(cleanedProperties.enabled); + assert.ok(cleanedProperties.notify); + assert.ok(cleanedProperties.required); + // The properties map itself must NOT have a "type" property injected + assert.equal(cleanedProperties.type, undefined); + assert.equal(cleanedProperties.required.type, "boolean"); +});