fix(tools): harden unified JSON value boundaries
This commit is contained in:
@@ -3,11 +3,12 @@
|
||||
/**
|
||||
* A value that round-trips losslessly through JSON: `null`, a boolean, a finite
|
||||
* number other than negative zero, a string, an array of such values, or a
|
||||
* plain object whose values are such values. TypeScript cannot distinguish
|
||||
* `-0` from `number`, so {@link isJsonValue} and {@link snapshotJsonValue}
|
||||
* enforce that last numeric detail at runtime. Use this type for a payload that
|
||||
* must survive session-log persistence and replay byte-identically — e.g. a
|
||||
* tool's private presentation `meta`.
|
||||
* plain object whose values are such values. Arrays may carry only their dense
|
||||
* indexed elements; extra own properties would be discarded by JSON. TypeScript
|
||||
* cannot distinguish `-0` from `number`, so {@link isJsonValue} and
|
||||
* {@link snapshotJsonValue} enforce these details at runtime. Use this type for
|
||||
* a payload that must survive session-log persistence and replay byte-identically
|
||||
* — e.g. a tool's private presentation `meta`.
|
||||
*/
|
||||
export type JsonValue = null | boolean | number | string | JsonValue[] | { [key: string]: JsonValue }
|
||||
|
||||
@@ -47,6 +48,10 @@ export function snapshotJsonValue<T>(value: T): T | undefined {
|
||||
if (Array.isArray(current)) {
|
||||
if (Object.getPrototypeOf(current) !== Array.prototype) return undefined
|
||||
const length = current.length
|
||||
// Every ordinary array owns `length`; dense indexed elements account
|
||||
// for the remaining keys. Anything else would be lost by JSON and by
|
||||
// structured clone, including symbols and non-enumerable properties.
|
||||
if (Reflect.ownKeys(current).length !== length + 1) return undefined
|
||||
const snapshot: JsonValue[] = []
|
||||
for (let index = 0; index < length; index++) {
|
||||
if (!Object.prototype.hasOwnProperty.call(current, index)) return undefined
|
||||
@@ -111,6 +116,7 @@ export function isJsonValue(value: unknown, seen: Set<object> = new Set()): bool
|
||||
try {
|
||||
if (Array.isArray(value)) {
|
||||
if (Object.getPrototypeOf(value) !== Array.prototype) return false
|
||||
if (Reflect.ownKeys(value).length !== value.length + 1) return false
|
||||
// Reject sparse arrays: a hole is skipped by `every`/`forEach` but
|
||||
// JSON.stringify writes it as `null`, so `[1, , 3]` would round-trip
|
||||
// lossily. Require every index 0..length-1 to be an OWN property.
|
||||
|
||||
@@ -63,12 +63,16 @@ describe('snapshotJsonValue', () => {
|
||||
expect(arrayReads).toBe(1)
|
||||
})
|
||||
|
||||
it('rejects exotic containers, sparse arrays, cycles, and invalid children', () => {
|
||||
it('rejects exotic containers, sparse or decorated arrays, cycles, and invalid children', () => {
|
||||
class ExoticObject {
|
||||
readonly value = 1
|
||||
}
|
||||
class ExoticArray extends Array<number> {}
|
||||
const sparse = new Array<number>(1)
|
||||
const decorated = [1]
|
||||
Object.defineProperty(decorated, 'extra', { value: true })
|
||||
const symbolDecorated = [1]
|
||||
Object.defineProperty(symbolDecorated, Symbol('extra'), { value: true })
|
||||
const cyclic: Record<string, unknown> = {}
|
||||
cyclic.self = cyclic
|
||||
|
||||
@@ -76,6 +80,8 @@ describe('snapshotJsonValue', () => {
|
||||
expect(snapshotJsonValue(new Map([['value', 1]]))).toBeUndefined()
|
||||
expect(snapshotJsonValue(new ExoticArray(1))).toBeUndefined()
|
||||
expect(snapshotJsonValue(sparse)).toBeUndefined()
|
||||
expect(snapshotJsonValue(decorated)).toBeUndefined()
|
||||
expect(snapshotJsonValue(symbolDecorated)).toBeUndefined()
|
||||
expect(snapshotJsonValue(cyclic)).toBeUndefined()
|
||||
expect(snapshotJsonValue([undefined])).toBeUndefined()
|
||||
expect(snapshotJsonValue({ value: undefined })).toBeUndefined()
|
||||
@@ -133,16 +139,21 @@ describe('isJsonValue', () => {
|
||||
expect(isJsonValue(nullPrototype)).toBe(true)
|
||||
})
|
||||
|
||||
it('rejects sparse arrays, invalid children, exotic objects, and cycles', () => {
|
||||
it('rejects sparse or decorated arrays, invalid children, exotic objects, and cycles', () => {
|
||||
class Exotic {
|
||||
readonly value = 1
|
||||
}
|
||||
class ExoticArray extends Array<number> {}
|
||||
const sparse = new Array<number>(1)
|
||||
const decorated = Object.assign([1], { extra: true })
|
||||
const symbolDecorated = [1]
|
||||
Object.defineProperty(symbolDecorated, Symbol('extra'), { value: true })
|
||||
const cyclic: Record<string, unknown> = {}
|
||||
cyclic.self = cyclic
|
||||
|
||||
expect(isJsonValue(sparse)).toBe(false)
|
||||
expect(isJsonValue(decorated)).toBe(false)
|
||||
expect(isJsonValue(symbolDecorated)).toBe(false)
|
||||
expect(isJsonValue(new ExoticArray(1))).toBe(false)
|
||||
expect(isJsonValue([undefined])).toBe(false)
|
||||
expect(isJsonValue({ value: undefined })).toBe(false)
|
||||
|
||||
@@ -178,7 +178,7 @@ Append-only; newly visible content follows the reusable request prefix and does
|
||||
|
||||
- **Concurrency policy is not an event seam** — `executionMode()` reads the resolved tool definition directly; plugins can only declare a classifier on definitions they own.
|
||||
- **`tools/pre-execute` deliberately cannot rewrite `exec.arguments`** — logged and rendered args would desync from what ran; the rewrite design is [a proposed Agent Note](../../../.agents/notes/proposed/feature/2026-06-30-pre-tool-input-rewrite.md).
|
||||
- **`defineTool`'s schema DSL is a deliberate subset** — string/number/boolean/object/array with string-only `enum`; `validateArgs` tolerates extra keys and preserves `default` as a model-visible JSON Schema annotation without applying it during validation; dynamic Cordis mounts may supply defaults even though first-party definitions do not, while raw-registered JSON-Schema tools validate their own input.
|
||||
- **Caller-defined subagent and workflow structured outputs remain object-rooted** — this is a consumer-level guard; the shared schema vocabulary supports every JSON root.
|
||||
- **`timeoutMs` on a definition is declarative only** — the registry never enforces deadlines; enforcement requires the `@deepseek-ai/dsh-timeout-policy` wrapper.
|
||||
- **Code Mode is TypeScript-only and the presentation mode is service-wide** — `mode: code`/`both` rejects prompt assembly unless `ctx.codeRuntime.language === 'typescript'`; scoped restrictions/shadows still choose each agent's visible bindings, but one tool cannot be native-only while another is code-only.
|
||||
- **Code Mode bindings return text only** — non-text content blocks in a sub-call result collapse to `[<type> content]` placeholders.
|
||||
|
||||
@@ -235,13 +235,21 @@ function checkSchemaNode(node: unknown, path: string, violations: string[], seen
|
||||
case 'boolean':
|
||||
case 'null': {
|
||||
const allowed = node.enum
|
||||
const enumValid = Array.isArray(allowed)
|
||||
&& allowed.length > 0
|
||||
&& allowed.every(entry => scalarMatches(schemaType, entry))
|
||||
if (Object.hasOwn(node, 'enum')) {
|
||||
if (!Array.isArray(allowed) || allowed.length === 0 || !allowed.every(entry => scalarMatches(schemaType, entry))) {
|
||||
if (!enumValid) {
|
||||
violations.push(`${path}.enum must be a non-empty array of ${schemaType} values`)
|
||||
}
|
||||
}
|
||||
if (Object.hasOwn(node, 'const') && !scalarMatches(schemaType, node.const)) {
|
||||
violations.push(`${path}.const must be a ${schemaType} value`)
|
||||
const constValid = scalarMatches(schemaType, node.const)
|
||||
if (Object.hasOwn(node, 'const')) {
|
||||
if (!constValid) {
|
||||
violations.push(`${path}.const must be a ${schemaType} value`)
|
||||
} else if (enumValid && !allowed.includes(node.const as JsonSchemaScalar)) {
|
||||
violations.push(`${path}.const must be one of ${path}.enum when both are declared`)
|
||||
}
|
||||
}
|
||||
break
|
||||
}
|
||||
@@ -300,8 +308,20 @@ function propertyPath(path: string, key: string): string {
|
||||
return path === '' ? key : `${path}.${key}`
|
||||
}
|
||||
|
||||
/** Collect value violations for one trusted schema node. */
|
||||
/** Contain hostile getters/proxies so validation remains total for arbitrary values. */
|
||||
function checkValue(node: JsonSchemaNode, value: unknown, path: string): string[] {
|
||||
if (node.type !== undefined && !(SCHEMA_TYPES as readonly unknown[]).includes(node.type)) {
|
||||
return checkValueUnchecked(node, value, path)
|
||||
}
|
||||
try {
|
||||
return checkValueUnchecked(node, value, path)
|
||||
} catch {
|
||||
return [`"${diagnosticPath(path)}" must be a lossless JSON value`]
|
||||
}
|
||||
}
|
||||
|
||||
/** Collect value violations for one trusted schema node after the exception boundary. */
|
||||
function checkValueUnchecked(node: JsonSchemaNode, value: unknown, path: string): string[] {
|
||||
if (node.oneOf !== undefined) {
|
||||
const matches = node.oneOf.filter(branch => checkValue(branch, value, path).length === 0).length
|
||||
return matches === 1 ? [] : [`"${diagnosticPath(path)}" must match exactly one oneOf branch (matched ${matches})`]
|
||||
|
||||
@@ -197,7 +197,12 @@ function compilePropertyMap(
|
||||
if (Object.hasOwn(property, 'required') && property.required !== true) {
|
||||
authorError(`${path}.${key}.required must be true when present`)
|
||||
}
|
||||
properties[key] = compileValueSchema(property, `${path}.${key}`, seen, true)
|
||||
Object.defineProperty(properties, key, {
|
||||
value: compileValueSchema(property, `${path}.${key}`, seen, true),
|
||||
enumerable: true,
|
||||
configurable: true,
|
||||
writable: true,
|
||||
})
|
||||
if (property.required === true) required.push(key)
|
||||
}
|
||||
return required.length > 0 ? { properties, required } : { properties }
|
||||
|
||||
@@ -160,6 +160,8 @@ describe('the enforced raw JSON Schema subset', () => {
|
||||
.toEqual(['schema.const must be a boolean value'])
|
||||
expect(violationsOf({ type: 'string', enum: undefined }))
|
||||
.toEqual(['schema.enum must be a non-empty array of string values'])
|
||||
expect(violationsOf({ type: 'string', enum: ['a'], const: 'b' }))
|
||||
.toEqual(['schema.const must be one of schema.enum when both are declared'])
|
||||
})
|
||||
|
||||
it('validates annotation types and lossless JSON payloads', () => {
|
||||
@@ -267,6 +269,21 @@ describe('validateJsonSchemaValue', () => {
|
||||
.toEqual(['"value" must be an object'])
|
||||
})
|
||||
|
||||
it('returns a violation instead of throwing for a container with a hostile getter', () => {
|
||||
const value = Object.defineProperty({}, 'answer', {
|
||||
enumerable: true,
|
||||
get() { throw new Error('getter exploded') },
|
||||
})
|
||||
const schema = asserted({
|
||||
type: 'object',
|
||||
properties: { answer: { type: 'integer' } },
|
||||
required: ['answer'],
|
||||
})
|
||||
|
||||
expect(validateJsonSchemaValue(schema, value))
|
||||
.toEqual(['"value" must be a lossless JSON value'])
|
||||
})
|
||||
|
||||
it('validates dense arrays per index and rejects lossy arrays', () => {
|
||||
const schema = asserted({ type: 'array', items: { type: 'integer' } })
|
||||
expect(validateJsonSchemaValue(schema, [1, 2])).toEqual([])
|
||||
|
||||
@@ -62,6 +62,7 @@ describe('the unified author schema DSL', () => {
|
||||
{ type: 'object' },
|
||||
{ oneOf: [{ type: 'string' }] },
|
||||
{ type: 'number', enum: ['1'] },
|
||||
{ type: 'string', enum: ['a'], const: 'b' },
|
||||
{ type: 'integer', const: 1.5 },
|
||||
{ type: 'json', default: undefined },
|
||||
{ type: 'array', items: { type: 'string', required: true } },
|
||||
@@ -91,6 +92,17 @@ describe('the unified author schema DSL', () => {
|
||||
expect(() => parameterSchemaSpecToJsonSchema(properties as ParameterSchemaSpec)).toThrow(/circular/)
|
||||
})
|
||||
|
||||
it('preserves a property literally named __proto__ as schema data', () => {
|
||||
const properties = Object.create(null) as ParameterSchemaSpec
|
||||
properties.__proto__ = { type: 'string', required: true }
|
||||
|
||||
const schema = parameterSchemaSpecToJsonSchema(properties)
|
||||
|
||||
expect(Object.hasOwn(schema.properties, '__proto__')).toBe(true)
|
||||
expect(schema.properties.__proto__).toEqual({ type: 'string' })
|
||||
expect(schema.required).toEqual(['__proto__'])
|
||||
})
|
||||
|
||||
it('infers scalar literals, arrays, objects, json, and exact-one unions', () => {
|
||||
expectTypeOf<InferValue<{ type: 'string'; enum: readonly ['a', 'b'] }>>().toEqualTypeOf<'a' | 'b'>()
|
||||
expectTypeOf<InferValue<{ type: 'number'; const: 1 }>>().toEqualTypeOf<1>()
|
||||
|
||||
Reference in New Issue
Block a user