Skip to content

Commit c1b532e

Browse files
mjerrisclaude
andcommitted
fix(schema): stop an unresolvable $ref silently disabling verb validation
`validateVerb('connect', {to, zzz_not_a_real_key})` returned `{"valid":true,"errors":[]}` — a config nobody validated came back clean. The bundled schema.json holds exactly one non-local `$ref`: `$defs/SWMLAction.SWML -> "SWMLObject.json"`, a sibling spec file that is not bundled. Ajv resolves refs EAGERLY, so compiling any verb whose `$defs` subtree reaches `SWMLAction` threw `can't resolve reference SWMLObject.json from id #`. `getVerbValidator`'s bare `catch` swallowed that and returned null, and `validateVerb` then fell through to `validateVerbLightweight`, which only checks required props — so a COMPILE FAILURE read as a PASS. The blast radius was never just `connect`: 8 of 39 verbs degraded this way, all via `... -> Action -> SWMLAction -> SWMLObject.json` — ai, ai_sidecar, amazon_bedrock, cond, connect, execute, join_conference, switch. Two independent fixes: 1. Ref policy (the root). Register a permissive placeholder for the schema's unbundled external ref on our own Ajv instance, so the ref RESOLVES and compilation succeeds. Every other constraint — crucially the `unevaluatedProperties` closure that rejects unknown keys — is then enforced normally; only the nested `SWML` payload goes unchecked. This is exactly how go's santhosh-tekuri validator already behaves, which is why go never showed the symptom. schema.json is NOT modified, vendored, or reinterpreted. 2. Loud degradation (the backstop). A compile failure is now recorded and reported as an error instead of falling through to the permissive lightweight check, so validation that did not happen can never again read as `valid:true`. New `compileFailedVerbs` / `precompileVerbValidators()` make the condition observable rather than silent. Both directions covered by 9 regression tests: a forbidden key is rejected and a legitimate config still passes on each of the 8 verbs, and the guard is proven by reinstating the pre-fix Ajv instance and asserting a refusal rather than a pass. Negative-controlled — with the ref policy disabled, 9 of them fail. Full suite: 133 files / 2835 tests passed. SURFACE's DRIFT/SURFACE-FRESH/ SURFACE-DIFF failures reproduce identically on clean main and are unrelated. Supplying the real SWMLObject.json remains an owner-held schema-artifact change (held under #223); afterwards the placeholder in UNBUNDLED_EXTERNAL_REFS can be dropped and the nested SWML payload becomes fully validated too. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PbNcuqH8o9WEoEnT24q4wB
1 parent 0e9c9bf commit c1b532e

2 files changed

Lines changed: 212 additions & 4 deletions

File tree

src/SchemaUtils.ts

Lines changed: 112 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,39 @@ import { createRequire } from 'module';
1010
import Ajv2020 from 'ajv/dist/2020.js';
1111
import type { ValidateFunction } from 'ajv';
1212

13+
/** The subset of the Ajv instance surface this module uses. */
14+
interface AjvInstance {
15+
compile: (s: object) => ValidateFunction;
16+
addSchema: (s: object, key: string) => void;
17+
}
18+
19+
/**
20+
* External `$ref` targets the bundled schema names but does NOT bundle.
21+
*
22+
* The bundled `schema.json` contains exactly one non-local `$ref`:
23+
* `$defs/SWMLAction.SWML -> "SWMLObject.json"`, a sibling spec file that is not
24+
* part of the bundle. Ajv resolves refs EAGERLY at compile time, so ANY verb
25+
* whose `$defs` subtree transitively reaches `SWMLAction` used to throw
26+
* `can't resolve reference SWMLObject.json from id #` — 8 of 39 verbs
27+
* (`ai`, `ai_sidecar`, `amazon_bedrock`, `cond`, `connect`, `execute`,
28+
* `join_conference`, `switch`), all via
29+
* `… -> Action -> SWMLAction -> SWMLObject.json`.
30+
*
31+
* Registering a permissive placeholder makes the ref RESOLVE to an
32+
* always-accept schema, so compilation succeeds and every OTHER constraint in
33+
* the verb — most importantly the `unevaluatedProperties` closure that rejects
34+
* unknown/misspelled keys — is enforced normally. Only the contents of the
35+
* nested `SWML` payload go unchecked, which is precisely the behaviour of go's
36+
* santhosh-tekuri validator, which tolerates the unresolved ref and still
37+
* rejects the surrounding unknown keys.
38+
*
39+
* This is an Ajv *ref-resolution policy* applied to our own Ajv instance. It
40+
* does not modify, vendor, or reinterpret `schema.json`; supplying the real
41+
* `SWMLObject.json` remains an owner-held schema-artifact change, after which
42+
* this placeholder can simply be dropped.
43+
*/
44+
const UNBUNDLED_EXTERNAL_REFS = ['SWMLObject.json'] as const;
45+
1346
/** Result of validating a SWML document. */
1447
export interface ValidationResult {
1548
/** Whether the document passed all validation checks. */
@@ -143,7 +176,10 @@ export class SchemaUtils {
143176
* `null` entry means a validator couldn't be built for that verb (fall back
144177
* to lightweight). Shared Ajv instance is created once. */
145178
private verbValidators: Map<string, ValidateFunction | null> = new Map();
146-
private ajv: { compile: (s: object) => ValidateFunction } | null | undefined = undefined;
179+
private ajv: AjvInstance | null | undefined = undefined;
180+
/** Verb names whose validator FAILED TO COMPILE (as opposed to verbs for which
181+
* no full validator is applicable). See {@link compileFailedVerbs}. */
182+
private compileFailures: Map<string, string> = new Map();
147183

148184
/**
149185
* Create a SchemaUtils instance.
@@ -181,6 +217,7 @@ export class SchemaUtils {
181217
loadSchema(): Record<string, unknown> | null {
182218
// A (re)load may change the schema; drop any compiled verb validators.
183219
this.verbValidators.clear();
220+
this.compileFailures.clear();
184221
this.ajv = undefined;
185222
// Try custom schema path first (mirrors Python's schema_path parameter)
186223
if (this._schemaPath) {
@@ -251,6 +288,37 @@ export class SchemaUtils {
251288
return true;
252289
}
253290

291+
/**
292+
* The verbs whose full validator FAILED TO COMPILE, mapped to the compiler
293+
* error — i.e. verbs for which {@link validateVerb} cannot actually validate.
294+
*
295+
* Only populated for verbs that have been validated at least once (validators
296+
* compile lazily). It exists so a compile failure is OBSERVABLE rather than
297+
* silent: previously such a failure fell through to the permissive lightweight
298+
* check and the caller received `{valid: true, errors: []}` for a config
299+
* nobody had validated.
300+
*
301+
* @returns A verb-name → compile-error map; empty when every compiled verb
302+
* validator built successfully.
303+
*/
304+
get compileFailedVerbs(): Record<string, string> {
305+
return Object.fromEntries(this.compileFailures);
306+
}
307+
308+
/**
309+
* Force every verb's full validator to compile, and report which ones failed.
310+
*
311+
* Validators are otherwise built lazily on first use, so a compile failure
312+
* stays invisible until some caller happens to validate that verb. Calling
313+
* this makes the whole set testable in one step.
314+
*
315+
* @returns A verb-name → compile-error map; empty when all verbs compile.
316+
*/
317+
precompileVerbValidators(): Record<string, string> {
318+
for (const verbName of this.verbs.keys()) this.getVerbValidator(verbName);
319+
return this.compileFailedVerbs;
320+
}
321+
254322
/**
255323
* Get all verb names defined in the schema.
256324
* @returns Array of verb names (e.g. ["answer", "ai", "hangup", ...]).
@@ -388,6 +456,24 @@ export class SchemaUtils {
388456
};
389457
}
390458

459+
// No full validator. Distinguish the two very different reasons:
460+
// (a) the verb's schema FAILED TO COMPILE — validation did not happen, and
461+
// reporting `valid: true` here would be a false clean bill of health.
462+
// Refuse loudly instead.
463+
// (b) no full validator is applicable (partial/mocked schema, verb absent
464+
// from `$defs`) — the lightweight required-props check is the intended,
465+
// documented behaviour.
466+
const compileError = this.compileFailures.get(verbName);
467+
if (compileError !== undefined) {
468+
return {
469+
valid: false,
470+
errors: [
471+
`Schema validation unavailable for '${verbName}': its schema failed to compile ` +
472+
`(${compileError}). The config was NOT validated; this is not a pass.`,
473+
],
474+
};
475+
}
476+
391477
return this.validateVerbLightweight(verbName, config);
392478
}
393479

@@ -397,7 +483,7 @@ export class SchemaUtils {
397483
* — a partial/mocked schema) so callers fall back to lightweight validation,
398484
* the TS mirror of Python's `_validate_verb_full` guard.
399485
*/
400-
private getAjv(): { compile: (s: object) => ValidateFunction } | null {
486+
private getAjv(): AjvInstance | null {
401487
if (this.ajv !== undefined) return this.ajv;
402488
const props = (this.schema?.['properties'] as Record<string, unknown> | undefined) ?? {};
403489
if (!this.schema || !('sections' in props)) {
@@ -411,11 +497,24 @@ export class SchemaUtils {
411497
// stderr with one warning per field). We validate structure/keys, not
412498
// formats.
413499
const AjvCtor = (Ajv2020 as unknown as { default?: typeof Ajv2020 }).default ?? Ajv2020;
414-
this.ajv = new (AjvCtor as new (o: object) => { compile: (s: object) => ValidateFunction })({
500+
const ajv = new (AjvCtor as new (o: object) => AjvInstance)({
415501
allErrors: true,
416502
strict: false,
417503
logger: false,
418504
});
505+
// Ref policy: resolve the schema's unbundled external `$ref`s to a
506+
// permissive placeholder so eager resolution cannot make compilation throw.
507+
// See UNBUNDLED_EXTERNAL_REFS for why this is a policy and not a schema edit.
508+
for (const ref of UNBUNDLED_EXTERNAL_REFS) {
509+
try {
510+
ajv.addSchema({ $id: ref }, ref);
511+
} catch {
512+
// A duplicate/invalid registration must not disable validation wholesale;
513+
// if the ref genuinely can't be satisfied the per-verb compile below will
514+
// fail and be reported LOUDLY rather than degrading silently.
515+
}
516+
}
517+
this.ajv = ajv;
419518
return this.ajv;
420519
}
421520

@@ -444,8 +543,17 @@ export class SchemaUtils {
444543
// it the raw const-union is enforced and the validator rejects values
445544
// the platform accepts (see applyWiden's own comment).
446545
built = ajv.compile(applyWiden({ $defs: defs, ...(verb.definition as object) }) as object);
447-
} catch {
546+
this.compileFailures.delete(verbName);
547+
} catch (e) {
548+
// A COMPILE FAILURE IS NOT "NOTHING TO VALIDATE". Record it so the
549+
// caller is never handed a silent pass for a verb nobody validated:
550+
// `validateVerb` reports it as an error rather than falling through to
551+
// the always-permissive lightweight check. (Regression guarded: the
552+
// unresolved external `$ref` SWMLObject.json used to make 8 verbs —
553+
// connect among them — accept arbitrary unknown keys with
554+
// `{valid:true,errors:[]}`.)
448555
built = null;
556+
this.compileFailures.set(verbName, (e as Error)?.message ?? String(e));
449557
}
450558
}
451559
this.verbValidators.set(verbName, built);

tests/SchemaUtils.verb.test.ts

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest';
22
import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
33
import { join } from 'node:path';
44
import { fileURLToPath } from 'node:url';
5+
import Ajv2020 from 'ajv/dist/2020.js';
56
import { SchemaUtils } from '../src/SchemaUtils.js';
67

78
/**
@@ -31,6 +32,22 @@ function schemaDefKeyForVerb(schemaDoc: Record<string, unknown>, verbName: strin
3132
throw new Error(`no $defs entry declares verb '${verbName}'`);
3233
}
3334

35+
/** Ajv's ESM/CJS interop default, as SchemaUtils itself resolves it. */
36+
const AjvCtor = ((Ajv2020 as unknown as { default?: typeof Ajv2020 }).default ??
37+
Ajv2020) as unknown as new (o: object) => object;
38+
39+
/** A minimal VALID config for each verb that reaches the unbundled external $ref. */
40+
const legitConfigs: Record<string, unknown> = {
41+
ai: { prompt: { text: 'hello' } },
42+
ai_sidecar: { prompt: { text: 'hello' }, lang: 'en' },
43+
amazon_bedrock: { prompt: { text: 'hello' } },
44+
cond: [{ when: 'x == 1', then: [{ hangup: {} }] }],
45+
connect: { to: 'sip:alice@example.com' },
46+
execute: { dest: 'main' },
47+
join_conference: { name: 'room1' },
48+
switch: { variable: 'x', case: { a: [{ hangup: {} }] } },
49+
};
50+
3451
describe('SchemaUtils — verb extraction and validation', () => {
3552
let schema: SchemaUtils;
3653

@@ -330,6 +347,89 @@ describe('SchemaUtils — verb extraction and validation', () => {
330347
});
331348
});
332349

350+
// A verb whose $defs subtree transitively reaches `SWMLAction` hits the
351+
// schema's one unbundled external `$ref` (`SWMLObject.json`). Ajv resolves
352+
// refs EAGERLY, so compiling those verbs used to THROW; `getVerbValidator`'s
353+
// bare catch swallowed it and `validateVerb` fell through to the permissive
354+
// lightweight check — so a config nobody validated came back
355+
// `{valid: true, errors: []}`. Measured on main @6a2aa09:
356+
// validateVerb('connect', {to, zzz_not_a_real_key}) -> {"valid":true,"errors":[]}
357+
// 8 of 39 verbs degraded this way, all via
358+
// `… -> Action -> SWMLAction -> SWMLObject.json`.
359+
describe('unbundled external $ref must not silently disable validation', () => {
360+
// Every verb that reached SWMLAction, i.e. the full blast radius.
361+
const previouslyDegraded = [
362+
'ai',
363+
'ai_sidecar',
364+
'amazon_bedrock',
365+
'cond',
366+
'connect',
367+
'execute',
368+
'join_conference',
369+
'switch',
370+
];
371+
372+
// Compiling all 39 verbs eagerly is a test-only sweep and costs ~2.5s: the 7
373+
// verbs that recurse through `SWMLMethod` are ~200-400ms each (they were
374+
// "free" before only because they threw immediately). Real callers compile
375+
// lazily and cache — one cold verb is ~330ms, warm is ~0.02ms.
376+
it('compiles a validator for EVERY verb in the schema', () => {
377+
expect(schema.precompileVerbValidators()).toEqual({});
378+
}, 30_000);
379+
380+
it('rejects an unknown key on connect (the reported case)', () => {
381+
const result = schema.validateVerb('connect', {
382+
to: 'sip:alice@example.com',
383+
zzz_not_a_real_key: 1,
384+
});
385+
expect(result.valid).toBe(false);
386+
});
387+
388+
it('still accepts a legitimate connect config', () => {
389+
expect(schema.validateVerb('connect', { to: 'sip:alice@example.com' }).valid).toBe(true);
390+
});
391+
392+
it.each(previouslyDegraded)('rejects an unknown key on %s', (verb) => {
393+
// `cond` takes an ARRAY of CondParams; inject the bogus key into an element.
394+
const config: Record<string, unknown> | unknown[] =
395+
verb === 'cond'
396+
? [{ when: 'x == 1', then: [{ hangup: {} }], zzz_not_a_real_key: 1 }]
397+
: { ...(legitConfigs[verb] as object), zzz_not_a_real_key: 1 };
398+
expect(schema.validateVerb(verb, config).valid).toBe(false);
399+
});
400+
401+
it.each(previouslyDegraded)('still accepts a legitimate %s config', (verb) => {
402+
expect(schema.validateVerb(verb, legitConfigs[verb]).valid).toBe(true);
403+
});
404+
405+
// The ref policy is the root fix; THIS is the backstop. Even if a verb's
406+
// schema someday fails to compile for an unrelated reason, the caller must
407+
// be told validation did not happen rather than handed a false pass.
408+
it('reports a refusal, never `valid: true`, when a verb fails to compile', () => {
409+
const su = new SchemaUtils();
410+
su.getVerbNames();
411+
// Reinstate the pre-fix condition: an Ajv instance with no placeholder
412+
// registered for the unbundled external ref.
413+
const internals = su as unknown as {
414+
ajv: unknown;
415+
verbValidators: Map<string, unknown>;
416+
compileFailures: Map<string, string>;
417+
};
418+
internals.verbValidators.clear();
419+
internals.compileFailures.clear();
420+
internals.ajv = new AjvCtor({ allErrors: true, strict: false, logger: false });
421+
422+
const result = su.validateVerb('connect', {
423+
to: 'sip:alice@example.com',
424+
zzz_not_a_real_key: 1,
425+
});
426+
expect(result.valid).toBe(false);
427+
expect(result.errors[0]).toContain('failed to compile');
428+
expect(result.errors[0]).toContain('NOT validated');
429+
expect(Object.keys(su.compileFailedVerbs)).toContain('connect');
430+
});
431+
});
432+
333433
describe('SWML_SKIP_SCHEMA_VALIDATION', () => {
334434
it('skips validation when skipValidation is true', () => {
335435
const skipped = new SchemaUtils({ skipValidation: true });

0 commit comments

Comments
 (0)