Skip to content

[Versioning] Fix fail-opens in schema compatibility detector #771

Description

Relevant area(s)

CI / build tooling — the schema-compatibility detector in scripts/versioning/lib/schema-compatibility.js (Node.js, runs on all platforms). Not backend-runtime; platform-independent.

Brief description of your issue

The structural schema-compatibility detector added in #731 (scripts/versioning/lib/schema-compatibility.js, detectBreaking(prev, next)) has four fail-open gaps: constructs that a downstream consumer would enforce as a restriction pass through as no finding, so detectBreaking returns [] when the change is actually breaking. Each violates the detector's fail-closed contract ("composing/normalizing never misses a restriction; anything unrecognized routes to manual review").

All four were raised as unresolved review threads on #731 and remain present at head 76f2e8fd (anchors below). None is triggerable by the current gate today — every MXC schema is generated draft-07 and never uses these keywords — but the detector is a general-purpose safety net intended to fail closed, and each fix is small and local.

Consolidated here (rather than four separate issues) because they share one root cause: keywords the detector doesn't model are compared by text equality / dropped, instead of routing to manual review.

Review threads:

  • L754 ($refWithSiblings forgeable sentinel)
  • L866 (unevaluatedProperties / unevaluatedItems)
  • L315 ($dynamicRef)
  • L8 ($schema dialect change)

Steps to reproduce

Each call below returns [] (no breaking change detected) even though the next schema rejects an instance the prev schema accepted.

1. Forgeable $refWithSiblings sentinel — anchor L754
diffNode treats any node literally carrying a $refWithSiblings key as the internal ref-composition marker and compares only its two array entries, ignoring every real assertion at that node. A user schema whose keyword is named $refWithSiblings collides with the sentinel:

detectBreaking({"$refWithSiblings":[true,true], type:"number"},
               {"$refWithSiblings":[true,true], type:"integer"}) // => []  (1.5 newly rejected, no finding)

Fix: use an unforgeable Symbol for the marker (L29) instead of a string property.

2. unevaluatedProperties / unevaluatedItems — anchor L866
In the property loop (L857-870) removing a declared property is treated as safe whenever additionalProperties is not false. But under an unchanged unevaluatedProperties:false, that property was marking its value evaluated, so removing it is a restriction:

detectBreaking({type:"object", properties:{a:true}, unevaluatedProperties:false},
               {type:"object", properties:{},       unevaluatedProperties:false}) // => []  ({a:1} accepted before, rejected after)

The same hole exists for contains removal under an unchanged unevaluatedItems:false. Fix: emit a manual-review finding whenever unevaluatedProperties/unevaluatedItems is present and an evaluation-producing applicator changes.

3. $dynamicRef not resolved — anchor L315
Ref normalization resolves $ref and skips $defs/definitions (L315-330), but $dynamicRef is left as an unrecognized keyword compared by text equality. Identical $dynamicRef text clears even when its $dynamicAnchor target tightens:

detectBreaking(
  {$defs:{T:{$dynamicAnchor:"T", type:"number"}},  properties:{a:{$dynamicRef:"#T"}}},
  {$defs:{T:{$dynamicAnchor:"T", type:"integer"}}, properties:{a:{$dynamicRef:"#T"}}}) // => []  ({a:1.5} newly rejected, no finding)

Fix: route any $dynamicRef to manual review (or resolve it scope-aware).

4. $schema dialect change dropped as annotation — anchor L8
$schema is dropped as an annotation, and ref-siblings are composed unconditionally on both sides (L315-330). Draft-07 ignores keywords beside $ref; 2020-12 applies them, so a draft-07 → 2020-12 bump can add an enforced sibling with no finding:

detectBreaking(
  {$schema:"http://json-schema.org/draft-07/schema#",      definitions:{T:{}}, $ref:"#/definitions/T", type:"string"},
  {$schema:"https://json-schema.org/draft/2020-12/schema", definitions:{T:{}}, $ref:"#/definitions/T", type:"string"}) // => []  (old accepts 42, new rejects it)

Fix: route a $schema dialect change to manual review.

Expected behavior

Each of the four detectBreaking(...) calls above should return a non-empty result — either a breaking-change finding or a manual-review finding. Any keyword the detector does not model ($dynamicRef, $schema dialect change, unevaluated*) should route to the manual-review / fail-closed path rather than clearing silently, and the internal ref-composition marker must not be forgeable by input.

Actual behavior

Each call returns [] — the newly-rejected instance produces no finding, so a breaking schema change would pass the compatibility gate. The detector fails open on these four constructs.

Metadata

Metadata

Labels

Type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions