From 8152c81e9c6ec3223514d4533731bc7ee55edcc9 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 3 Aug 2026 22:50:58 -0700 Subject: [PATCH 1/5] Extract the composition-aware instance validator so a second gate can reuse it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `instance_errors` and the `merged_constraints` walk beneath it were top-level methods inside scripts/check-fixture-coverage.rb, which runs its whole check on load — so a second gate could not require them without also running the fixture guard. Moved verbatim into scripts/schema_instance_validator.rb, in the scripts/bc3_route_normalizer.rb style: one definition, two callers, no parallel validator to drift. `module_function` plus a top-level `include` keeps every existing call site receiverless and unchanged, so this is behaviour-preserving; the guard's own self-test (1 positive + 8 negative + 20 synthetic cases) is what says so. What stays in the fixture guard is what is about the fixture guard: the concrete-instance rule and the #408 rich-text-emitter inventory. --- scripts/check-fixture-coverage.rb | 256 +++----------------------- scripts/schema_instance_validator.rb | 259 +++++++++++++++++++++++++++ 2 files changed, 279 insertions(+), 236 deletions(-) create mode 100644 scripts/schema_instance_validator.rb diff --git a/scripts/check-fixture-coverage.rb b/scripts/check-fixture-coverage.rb index db00efec13..304fc8f710 100755 --- a/scripts/check-fixture-coverage.rb +++ b/scripts/check-fixture-coverage.rb @@ -27,39 +27,31 @@ # components, and do not overlap `covered_schemas`. # # Uses conformance/runner/ruby/schema-walker.rb only for `find_response_schema` -# (operation-entry response-schema lookup). The required/type/nullability walk is -# implemented here because it must be composition-aware, which the walker is not. -# Wired into `make check` in the scripts/validate-api-gaps.rb style (stdlib only). +# (operation-entry response-schema lookup). The required/type/nullability walk +# cannot come from the walker because it must be composition-aware, which the +# walker is not; it lives in scripts/schema_instance_validator.rb, extracted so +# scripts/check-projected-examples.rb reuses the same walk instead of growing a +# parallel one. Wired into `make check` in the scripts/validate-api-gaps.rb style +# (stdlib only). # # Paths default to the repo layout but honour FIXTURE_MANIFEST / FIXTURE_OPENAPI # / FIXTURE_DIR env overrides so the negative-case self-test # (scripts/test-check-fixture-coverage.rb) can point it at crafted inputs. # -# --- Scope: a deliberately PARTIAL structural validator ------------------------ -# -# This is NOT a complete JSON-Schema / OpenAPI validator. It validates the subset -# that keeps a fixture structurally faithful to the generated types: -# * required-field presence -# * declared types (with integer/number integrality) and nullability -# * arrays and array-element typing/nullability -# * $ref (incl. 3.1 $ref-with-siblings) and allOf conjunction (required unioned; -# duplicate properties and array `items` conjoined; type constraints -# intersected; nullability = every part permits null) -# * anyOf/oneOf as AT-LEAST-ONE (the value must satisfy some branch; a group is -# nullable only if a branch is) -# -# Intentionally NOT implemented (out of scope unless a case is reached by the -# actual generated schemas): exact-one `oneOf` selection, `enum`, `const`, -# discriminators, `pattern`, `format`, numeric bounds, `additionalProperties`, -# `uniqueItems`, and other assertion keywords. Findings about these are declined -# unless they affect the current generated `openapi.json` or this documented -# subset. +# The instance walk is a deliberately PARTIAL structural validator — see the +# scope note in scripts/schema_instance_validator.rb for exactly what it does and +# does not assert. require "json" require "yaml" require "set" require_relative "../conformance/runner/ruby/schema-walker" +require_relative "schema_instance_validator" + +# Receiverless `instance_errors(...)` / `merged_constraints(...)` / `ref_name(...)` +# below, exactly as when they were defined in this file. +include SchemaInstanceValidator # rubocop:disable Style/MixinUsage PROJECT_ROOT = File.expand_path("..", __dir__) MANIFEST_FILE = ENV.fetch("FIXTURE_MANIFEST", File.join(PROJECT_ROOT, "spec/fixtures/manifest.yaml")) @@ -112,220 +104,12 @@ def resolve_pointer(doc, pointer) end # --- Schema helpers ------------------------------------------------------------ - -# Ruby name of a `$ref`, or nil. -def ref_name(schema) - return nil unless schema.is_a?(Hash) && schema["$ref"].is_a?(String) - - schema["$ref"].match(%r{/components/schemas/(.+)\z})&.captures&.first -end - -# Returns [Set(declared non-null json-type strings), nullable?] for a single -# schema node (no traversal). Handles OpenAPI 3.1 null-union (`type:[X,"null"]`) -# and 3.0 `nullable:true`. An empty set means the node declares no type. -def allowed_types(schema) - return [Set.new, false] unless schema.is_a?(Hash) - - t = schema["type"] - case t - when Array - members = t.compact - non_null = members - ["null"] - # A union of only ["null"] still constrains the value to null — keep "null" - # as the type so a non-null value fails (an empty set would be unconstrained). - types = non_null.empty? ? Set.new(["null"]) : Set.new(non_null) - [types, members.include?("null")] - when String - # OpenAPI 3.1 scalar null type: the value must BE null — a real "null" type - # constraint (so a non-null value fails), and it is nullable. Returning an - # empty type set would wrongly impose no constraint at all. - return [Set.new(["null"]), true] if t == "null" - - [Set.new([t]), schema["nullable"] == true] - else - [Set.new, schema["nullable"] == true] - end -end - -def json_type(value) - case value - when Hash then "object" - when Array then "array" - when String then "string" - when true, false then "boolean" - when Integer then "integer" - when Float then "number" - else "null" - end -end - -# True when `value`'s JSON type satisfies the declared `types`. integer/number -# interchange with one guard: a float supplied for an integer-only field passes -# only when it is mathematically integral (so FlexInt `1024.0` passes but `1.5` -# fails). -def type_matches?(types, value) - return true if types.empty? - - actual = json_type(value) - return true if types.include?(actual) - - if actual == "number" && types.include?("integer") && !types.include?("number") - return value.is_a?(Float) && value.finite? && value == value.truncate - end - return true if actual == "integer" && types.include?("number") - - false -end - -# Merges a schema's effective constraints across `$ref` (including 3.1 -# `$ref`-with-siblings) and `allOf`, returning -# [required(Array), properties(Hash), types(Set), nullable(bool), items(schema), -# alt_groups(Array of branch-arrays), type_sets(Array of Sets)]. -# `allOf` is a conjunction: required is unioned, and properties AND array `items` -# constrained by more than one branch are conjoined (allOf-wrapped) so a value -# must satisfy all of them; `type_sets` holds each part's declared type-set (a -# value must match every one). `anyOf`/`oneOf` are alternatives: their branches -# are NOT merged (that would over-require) but each group is collected so -# validation can require at least one branch to match — including groups -# inherited through `$ref` and `allOf`. `visited` (component names) + depth guard -# terminate reference/composition cycles. -def merged_constraints(schema, components, visited = Set.new, depth = 0) - req = [] - props = {} - types = Set.new # union of all declared types (for messages + concrete_for?) - type_sets = [] # per-conjunctive-part declared type-sets — a value must - # satisfy EVERY one (allOf/$ref are a conjunction, so their - # type constraints INTERSECT, not union). - # `$ref`(+siblings) and allOf form a CONJUNCTION: null is allowed only if every - # part allows it, so a part that imposes a non-nullable type FORBIDS null. We - # accumulate `forbids_null` (OR) and return its negation as the nullable flag. - forbids_null = false - items = nil - alt_groups = [] - return [req, props, types, true, items, alt_groups, type_sets] if depth > 40 || !schema.is_a?(Hash) - - # When the same property (or array `items`) is constrained by more than one - # conjunctive part (e.g. declared in two allOf branches), conjoin the schemas - # so the value must satisfy ALL of them — not just the first seen. - add_prop = lambda do |k, v| - props[k] = props.key?(k) ? { "allOf" => [props[k], v] } : v - end - add_items = lambda do |i| - items = items ? { "allOf" => [items, i] } : i - end - - absorb = lambda do |sub| - r2, p2, t2, sub_nullable, i2, a2, ts2 = merged_constraints(sub, components, visited, depth + 1) - req.concat(r2) - p2.each { |k, v| add_prop.call(k, v) } - types.merge(t2) - type_sets.concat(ts2) - forbids_null ||= !sub_nullable - add_items.call(i2) if i2 - alt_groups.concat(a2) - end - - name = ref_name(schema) - if name && !visited.include?(name) - visited << name - absorb.call(components[name]) - # fall through to local keywords (OpenAPI 3.1 permits $ref siblings) - end - - t, nn = allowed_types(schema) - types.merge(t) - type_sets << t unless t.empty? - # A node that imposes a concrete type but does not permit null forbids null. - forbids_null ||= (!t.empty? && !nn) - (schema["properties"] || {}).each { |k, v| add_prop.call(k, v) } - (schema["required"] || []).each { |r| req << r } - add_items.call(schema["items"]) if schema["items"] - (schema["allOf"] || []).each { |sub| absorb.call(sub) } - %w[anyOf oneOf].each do |key| - alt_groups << schema[key] if schema[key].is_a?(Array) && !schema[key].empty? - end - - # An anyOf/oneOf group (local or inherited via $ref/allOf) permits null only if - # at least one branch does; if every branch forbids null, the group forbids it - # (the value must satisfy some branch). Conjoin that with the surrounding - # $ref/allOf constraints. Branch nullability is computed with a FRESH visited - # set so outer traversal state can't short-circuit it. - alt_groups.each do |branches| - group_allows_null = branches.any? do |branch| - _, _, _, branch_nullable, = merged_constraints(branch, components, Set.new, depth + 1) - branch_nullable - end - forbids_null ||= !group_allows_null - end - - [req, props, types, !forbids_null, items, alt_groups, type_sets] -end - -# Composition-aware validation of `value` against `schema`. Reports -# (path-tagged) errors for a missing required field, a required field present as -# null against a non-nullable schema, a null array element against a non-nullable -# item schema, and a present value whose JSON type contradicts the declared type. -# Optional object-field nulls are tolerated: the Smithy-derived OpenAPI -# under-marks some nullable optionals (e.g. Person.bio/location are `type:string` -# yet the wire sends null), so flagging them would be a false positive. -def instance_errors(prefix, value, schema, components, depth = 0) - return [] if depth > 60 - return [] if value.nil? # optional-null tolerated; required-/element-null handled in context - - req, props, _types, _nullable, items, alt_groups, type_sets = merged_constraints(schema, components) - - # The value must satisfy EVERY conjunctive part's declared type (allOf/$ref - # intersect their type constraints — a value matching only one contradictory - # branch fails). - type_sets.each do |ts| - next if type_matches?(ts, value) - - label = prefix.empty? ? "(root)" : prefix - return ["#{label}: expected #{ts.to_a.sort.join('|')}, got #{json_type(value)}"] - end - - errs = [] - - # anyOf/oneOf: the value must satisfy at least one branch of each group - # (oneOf is validated as "at least one" — enforcing exactly-one would need full - # discriminator/enum/const validation to avoid false positives). - alt_groups.each do |branches| - next if branches.any? { |branch| instance_errors(prefix, value, branch, components, depth + 1).empty? } - - label = prefix.empty? ? "(root)" : prefix - errs << "#{label}: value matches none of the #{branches.length} allowed alternatives (anyOf/oneOf)" - end - - if value.is_a?(Hash) - req.uniq.each do |rk| - field = prefix.empty? ? rk : "#{prefix}/#{rk}" - if !value.key?(rk) - errs << "missing required field `#{field}`" - elsif value[rk].nil? - _, _, _, field_nullable, = merged_constraints(props[rk] || {}, components) - errs << "#{field}: required field is null but its schema is not nullable" unless field_nullable - end - end - value.each do |k, v| - next unless props.key?(k) - - child = prefix.empty? ? k : "#{prefix}/#{k}" - errs.concat(instance_errors(child, v, props[k], components, depth + 1)) - end - elsif value.is_a?(Array) && items - _, _, _, item_nullable, = merged_constraints(items, components) - value.each_with_index do |item, i| - ip = "#{prefix}[#{i}]" - if item.nil? - errs << "#{ip}: null array element but the item schema is not nullable" unless item_nullable - next - end - errs.concat(instance_errors(ip, item, items, components, depth + 1)) - end - end - - errs -end +# +# The generic walk (`ref_name`, `allowed_types`, `json_type`, `type_matches?`, +# `merged_constraints`, `instance_errors`) lives in +# scripts/schema_instance_validator.rb, included above. What remains here is the +# part that is about THIS guard: the concrete-instance rule and the #408 +# rich-text-emitter inventory. def concrete_for?(schema_name, instance, components) _, _, types, = merged_constraints({ "$ref" => "#/components/schemas/#{schema_name}" }, components) diff --git a/scripts/schema_instance_validator.rb b/scripts/schema_instance_validator.rb new file mode 100644 index 0000000000..5d703cbd56 --- /dev/null +++ b/scripts/schema_instance_validator.rb @@ -0,0 +1,259 @@ +# frozen_string_literal: true + +require "set" + +# Composition-aware structural validation of a JSON instance against a schema +# from the generated `openapi.json`. +# +# Extracted from scripts/check-fixture-coverage.rb so a second gate can reuse it +# rather than grow a parallel validator that drifts. Two callers today: +# +# * check-fixture-coverage.rb — fixture instances vs their declared schema +# * check-projected-examples.rb — the spec's own published `examples` vs the +# schema they sit under, which is where `jsonAdd`-injected `required` entries +# first become checkable (Smithy validates `@examples` against the *Smithy* +# model, before the projection adds anything). +# +# Both scripts are stdlib-only and read the same generated document, so the +# validator carries no I/O and no configuration — pass it parsed JSON. +# +# --- Scope: a deliberately PARTIAL structural validator ------------------------ +# +# This is NOT a complete JSON-Schema / OpenAPI validator. It validates the subset +# that keeps an instance structurally faithful to the generated types: +# * required-field presence +# * declared types (with integer/number integrality) and nullability +# * arrays and array-element typing/nullability +# * $ref (incl. 3.1 $ref-with-siblings) and allOf conjunction (required unioned; +# duplicate properties and array `items` conjoined; type constraints +# intersected; nullability = every part permits null) +# * anyOf/oneOf as AT-LEAST-ONE (the value must satisfy some branch; a group is +# nullable only if a branch is) +# +# Intentionally NOT implemented (out of scope unless a case is reached by the +# actual generated schemas): exact-one `oneOf` selection, `enum`, `const`, +# discriminators, `pattern`, `format`, numeric bounds, `additionalProperties`, +# `uniqueItems`, and other assertion keywords. Findings about these are declined +# unless they affect the current generated `openapi.json` or this documented +# subset. +# +# `module_function` + a top-level `include` in the calling script keeps the +# receiverless call style the extraction moved out of; `SchemaInstanceValidator.x` +# works too. +module SchemaInstanceValidator + module_function + + # Ruby name of a `$ref`, or nil. + def ref_name(schema) + return nil unless schema.is_a?(Hash) && schema["$ref"].is_a?(String) + + schema["$ref"].match(%r{/components/schemas/(.+)\z})&.captures&.first + end + + # Returns [Set(declared non-null json-type strings), nullable?] for a single + # schema node (no traversal). Handles OpenAPI 3.1 null-union (`type:[X,"null"]`) + # and 3.0 `nullable:true`. An empty set means the node declares no type. + def allowed_types(schema) + return [Set.new, false] unless schema.is_a?(Hash) + + t = schema["type"] + case t + when Array + members = t.compact + non_null = members - ["null"] + # A union of only ["null"] still constrains the value to null — keep "null" + # as the type so a non-null value fails (an empty set would be unconstrained). + types = non_null.empty? ? Set.new(["null"]) : Set.new(non_null) + [types, members.include?("null")] + when String + # OpenAPI 3.1 scalar null type: the value must BE null — a real "null" type + # constraint (so a non-null value fails), and it is nullable. Returning an + # empty type set would wrongly impose no constraint at all. + return [Set.new(["null"]), true] if t == "null" + + [Set.new([t]), schema["nullable"] == true] + else + [Set.new, schema["nullable"] == true] + end + end + + def json_type(value) + case value + when Hash then "object" + when Array then "array" + when String then "string" + when true, false then "boolean" + when Integer then "integer" + when Float then "number" + else "null" + end + end + + # True when `value`'s JSON type satisfies the declared `types`. integer/number + # interchange with one guard: a float supplied for an integer-only field passes + # only when it is mathematically integral (so FlexInt `1024.0` passes but `1.5` + # fails). + def type_matches?(types, value) + return true if types.empty? + + actual = json_type(value) + return true if types.include?(actual) + + if actual == "number" && types.include?("integer") && !types.include?("number") + return value.is_a?(Float) && value.finite? && value == value.truncate + end + return true if actual == "integer" && types.include?("number") + + false + end + + # Merges a schema's effective constraints across `$ref` (including 3.1 + # `$ref`-with-siblings) and `allOf`, returning + # [required(Array), properties(Hash), types(Set), nullable(bool), items(schema), + # alt_groups(Array of branch-arrays), type_sets(Array of Sets)]. + # `allOf` is a conjunction: required is unioned, and properties AND array `items` + # constrained by more than one branch are conjoined (allOf-wrapped) so a value + # must satisfy all of them; `type_sets` holds each part's declared type-set (a + # value must match every one). `anyOf`/`oneOf` are alternatives: their branches + # are NOT merged (that would over-require) but each group is collected so + # validation can require at least one branch to match — including groups + # inherited through `$ref` and `allOf`. `visited` (component names) + depth guard + # terminate reference/composition cycles. + def merged_constraints(schema, components, visited = Set.new, depth = 0) + req = [] + props = {} + types = Set.new # union of all declared types (for messages + concrete_for?) + type_sets = [] # per-conjunctive-part declared type-sets — a value must + # satisfy EVERY one (allOf/$ref are a conjunction, so their + # type constraints INTERSECT, not union). + # `$ref`(+siblings) and allOf form a CONJUNCTION: null is allowed only if every + # part allows it, so a part that imposes a non-nullable type FORBIDS null. We + # accumulate `forbids_null` (OR) and return its negation as the nullable flag. + forbids_null = false + items = nil + alt_groups = [] + return [req, props, types, true, items, alt_groups, type_sets] if depth > 40 || !schema.is_a?(Hash) + + # When the same property (or array `items`) is constrained by more than one + # conjunctive part (e.g. declared in two allOf branches), conjoin the schemas + # so the value must satisfy ALL of them — not just the first seen. + add_prop = lambda do |k, v| + props[k] = props.key?(k) ? { "allOf" => [props[k], v] } : v + end + add_items = lambda do |i| + items = items ? { "allOf" => [items, i] } : i + end + + absorb = lambda do |sub| + r2, p2, t2, sub_nullable, i2, a2, ts2 = merged_constraints(sub, components, visited, depth + 1) + req.concat(r2) + p2.each { |k, v| add_prop.call(k, v) } + types.merge(t2) + type_sets.concat(ts2) + forbids_null ||= !sub_nullable + add_items.call(i2) if i2 + alt_groups.concat(a2) + end + + name = ref_name(schema) + if name && !visited.include?(name) + visited << name + absorb.call(components[name]) + # fall through to local keywords (OpenAPI 3.1 permits $ref siblings) + end + + t, nn = allowed_types(schema) + types.merge(t) + type_sets << t unless t.empty? + # A node that imposes a concrete type but does not permit null forbids null. + forbids_null ||= (!t.empty? && !nn) + (schema["properties"] || {}).each { |k, v| add_prop.call(k, v) } + (schema["required"] || []).each { |r| req << r } + add_items.call(schema["items"]) if schema["items"] + (schema["allOf"] || []).each { |sub| absorb.call(sub) } + %w[anyOf oneOf].each do |key| + alt_groups << schema[key] if schema[key].is_a?(Array) && !schema[key].empty? + end + + # An anyOf/oneOf group (local or inherited via $ref/allOf) permits null only if + # at least one branch does; if every branch forbids null, the group forbids it + # (the value must satisfy some branch). Conjoin that with the surrounding + # $ref/allOf constraints. Branch nullability is computed with a FRESH visited + # set so outer traversal state can't short-circuit it. + alt_groups.each do |branches| + group_allows_null = branches.any? do |branch| + _, _, _, branch_nullable, = merged_constraints(branch, components, Set.new, depth + 1) + branch_nullable + end + forbids_null ||= !group_allows_null + end + + [req, props, types, !forbids_null, items, alt_groups, type_sets] + end + + # Composition-aware validation of `value` against `schema`. Reports + # (path-tagged) errors for a missing required field, a required field present as + # null against a non-nullable schema, a null array element against a non-nullable + # item schema, and a present value whose JSON type contradicts the declared type. + # Optional object-field nulls are tolerated: the Smithy-derived OpenAPI + # under-marks some nullable optionals (e.g. Person.bio/location are `type:string` + # yet the wire sends null), so flagging them would be a false positive. + def instance_errors(prefix, value, schema, components, depth = 0) + return [] if depth > 60 + return [] if value.nil? # optional-null tolerated; required-/element-null handled in context + + req, props, _types, _nullable, items, alt_groups, type_sets = merged_constraints(schema, components) + + # The value must satisfy EVERY conjunctive part's declared type (allOf/$ref + # intersect their type constraints — a value matching only one contradictory + # branch fails). + type_sets.each do |ts| + next if type_matches?(ts, value) + + label = prefix.empty? ? "(root)" : prefix + return ["#{label}: expected #{ts.to_a.sort.join('|')}, got #{json_type(value)}"] + end + + errs = [] + + # anyOf/oneOf: the value must satisfy at least one branch of each group + # (oneOf is validated as "at least one" — enforcing exactly-one would need full + # discriminator/enum/const validation to avoid false positives). + alt_groups.each do |branches| + next if branches.any? { |branch| instance_errors(prefix, value, branch, components, depth + 1).empty? } + + label = prefix.empty? ? "(root)" : prefix + errs << "#{label}: value matches none of the #{branches.length} allowed alternatives (anyOf/oneOf)" + end + + if value.is_a?(Hash) + req.uniq.each do |rk| + field = prefix.empty? ? rk : "#{prefix}/#{rk}" + if !value.key?(rk) + errs << "missing required field `#{field}`" + elsif value[rk].nil? + _, _, _, field_nullable, = merged_constraints(props[rk] || {}, components) + errs << "#{field}: required field is null but its schema is not nullable" unless field_nullable + end + end + value.each do |k, v| + next unless props.key?(k) + + child = prefix.empty? ? k : "#{prefix}/#{k}" + errs.concat(instance_errors(child, v, props[k], components, depth + 1)) + end + elsif value.is_a?(Array) && items + _, _, _, item_nullable, = merged_constraints(items, components) + value.each_with_index do |item, i| + ip = "#{prefix}[#{i}]" + if item.nil? + errs << "#{ip}: null array element but the item schema is not nullable" unless item_nullable + next + end + errs.concat(instance_errors(ip, item, items, components, depth + 1)) + end + end + + errs + end +end From bb30878eb2f84521842bf1d50a9c9b99103a2505 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 3 Aug 2026 22:51:13 -0700 Subject: [PATCH 2/5] Validate the projected examples against the schema the projection publishes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `spec/smithy-build.json`'s `jsonAdd` can append to a schema's `required` array in the OpenAPI projection — the route #630/#637 took for `Todolist.color`, because the shape carries `@examples` and Smithy cannot express a `null` in an example for a String. `smithy validate` checks `@examples` against the *Smithy* model, where the member is still natively optional; the requiredness arrives afterwards. Nothing validated the projected examples against the projected schema, so #637 shipped both `GetTodolistOrGroup` response examples without `color` while the schema declared it required, and a bot reviewer caught it rather than CI. scripts/check-projected-examples.rb walks every example openapi.json publishes — response, request-body and parameter — and validates each against the sibling schema through the extracted composition-aware walk. `examples` maps hold Example Objects (value under `value`, `$ref`s into components/examples resolved, `externalValue` reported as skipped rather than fetched) while the singular `example` IS the value; conflating the two would silently unwrap any payload with a field named `value`. It compares exactly what is published against exactly the schema it is published under, and unwraps nothing. That is only possible because #644 landed first: the bare-response mappers used to rewrite a single-property `*ResponseContent` into the bare payload while leaving the wrapper on the example, and `BareResponseExampleMapper` now mirrors the unwrapping onto examples so the two agree. A gate that also accepted the wrapped shape could not tell a correct example from a regression in that mapper — so this gate guards it too, and self-test case 7 puts the wrapper back and asserts the run goes red. The gate refuses to pass vacuously: validating zero response examples — the class the seam lives in — is reported as a failure, not a green run. The self-test drives 1 positive + 14 negative/skip cases through the real checker via `PROJECTED_EXAMPLES_OPENAPI`. Its header carries the measured mutation matrix: which cases go red when each guard is removed from a scratch copy, including a mutation that unwires the validator while leaving every walk intact, so coverage and judgement are pinned separately. Closes #638 --- scripts/check-projected-examples.rb | 259 +++++++++++++++ scripts/test-check-projected-examples.rb | 383 +++++++++++++++++++++++ 2 files changed, 642 insertions(+) create mode 100755 scripts/check-projected-examples.rb create mode 100755 scripts/test-check-projected-examples.rb diff --git a/scripts/check-projected-examples.rb b/scripts/check-projected-examples.rb new file mode 100755 index 0000000000..eae4768d3c --- /dev/null +++ b/scripts/check-projected-examples.rb @@ -0,0 +1,259 @@ +#!/usr/bin/env ruby +# frozen_string_literal: true + +# Projected-example guard. +# +# Validates every example published in the generated `openapi.json` against the +# schema it sits under, in the PROJECTION — which is the one place nothing was +# checking. +# +# --- The seam this exists for -------------------------------------------------- +# +# `spec/smithy-build.json`'s `jsonAdd` can append to a schema's `required` array +# in the OpenAPI projection: +# +# "/components/schemas/Todolist/required/-": "color" +# +# That route exists because `Todolist` carries `@examples` and Smithy cannot put +# a `null` into an example for a String shape, so native `@required` on a +# required-AND-nullable member fails Smithy validation (#630, #637). +# +# `smithy validate` checks `@examples` against the SMITHY model, where the member +# is still natively optional. The projection adds the requiredness afterwards. +# So a projection-added required field can be missing from a published example +# and every gate stays green. That is not hypothetical: #637 shipped both +# `GetTodolistOrGroup` response examples without `color` while the schema +# declared it required, and a bot reviewer — not CI — caught it (fixed in +# 12b02cda8 by injecting the example values through `jsonAdd` too). See #638. +# +# --- What is checked ----------------------------------------------------------- +# +# Every `examples` map entry and every singular `example` under: +# * responses[code].content[mediaType] +# * requestBody.content[mediaType] +# * parameters[] (operation-level and path-item-level) +# is validated against the sibling `schema`, using the same composition-aware +# walk the fixture guard uses (scripts/schema_instance_validator.rb): required +# presence, declared type, nullability, arrays, `$ref`/`allOf` conjunction, +# `anyOf`/`oneOf` as at-least-one. Example objects given as +# `$ref: '#/components/examples/X'` are resolved first. +# +# --- The response envelope, and why there is nothing to do about it ------------- +# +# The mappers in spec/smithy-bare-arrays rewrite a single-property +# `*ResponseContent` schema into the bare payload, because BC3 returns bare +# bodies while Smithy's restJson1 requires a wrapper structure. Until #644 they +# rewrote the SCHEMA only, so the published example kept the wrapper member +# Smithy emits from the `@examples` `output` node: +# +# schema: {"$ref": ".../Todolist"} +# example: {"result": { ...todolist... }} <- before #644 +# +# #648's `BareResponseExampleMapper` mirrors that unwrapping onto the examples, +# so the two now agree and this gate compares them directly. No unwrapping here, +# and deliberately none: a gate that ALSO accepted the wrapped shape could not +# tell a correct example from a regression in that mapper. Comparing exactly what +# is published against exactly the schema it is published under is the whole +# claim, and it means this gate now guards `BareResponseExampleMapper` too — put +# the wrapper back and the run goes red. +# +# --- Known blind spot ---------------------------------------------------------- +# +# This validates the examples that EXIST. A `required/-` pointer aimed at a +# schema that no example reaches is still unvalidated — the gate cannot fail on +# an example nobody wrote. The liveness floor below keeps that from silently +# becoming true of the whole gate, but it is not example coverage. Adding +# examples is the spec's job; this is the gate that makes them mean something. +# +# --- Running ------------------------------------------------------------------- +# +# `ruby scripts/check-projected-examples.rb`, or `make check-projected-examples` +# (which also runs the self-test). `PROJECTED_EXAMPLES_OPENAPI` overrides the +# input document so scripts/test-check-projected-examples.rb can drive crafted +# specs through this exact checker. + +require "json" + +require_relative "schema_instance_validator" + +PROJECT_ROOT = File.expand_path("..", __dir__) +OPENAPI_FILE = ENV.fetch("PROJECTED_EXAMPLES_OPENAPI", File.join(PROJECT_ROOT, "openapi.json")) + +# HTTP methods an OpenAPI path item may carry. Anything else at that level +# (`parameters`, `summary`, `$ref`, `x-*`) is not an operation. +HTTP_METHODS = %w[get put post delete options head patch trace].freeze + +# Read text as UTF-8 regardless of the process locale (LC_ALL=C would otherwise +# read as US-ASCII and choke on the spec's UTF-8 bytes). +def read_utf8(path) + File.read(path, encoding: "UTF-8") +end + +# Every example a media-type / parameter node publishes, as +# [[label, status, payload], ...] where status is one of: +# +# :value payload is the example value, ready to validate +# :skip payload is the reason there is nothing to validate +# :error payload is the reason this example cannot be checked at all +# +# OpenAPI allows two spellings and they mean different things. `examples` is a +# map of Example OBJECTS — the value lives under `value` (or is remote, under +# `externalValue`) — while the singular `example` IS the value. Conflating them +# would silently unwrap any example payload that happens to have a field named +# `value`. The generator emits only `examples` today; handling both means a +# future singular `example` cannot slip in unchecked. +def example_entries(node, doc) + entries = [] + examples = node["examples"] + if examples.is_a?(Hash) + examples.each { |name, example| entries << [name, *example_object_value(example, doc)] } + end + entries << ["(example)", :value, node["example"]] if node.key?("example") + entries +end + +# Unpacks one Example Object, resolving `$ref: '#/components/examples/X'` first. +def example_object_value(example, doc) + if example.is_a?(Hash) && example["$ref"].is_a?(String) + name = example["$ref"].match(%r{\A#/components/examples/(.+)\z})&.captures&.first + example = name && doc.dig("components", "examples", name) + return [:error, "`$ref` does not resolve to an entry in components/examples"] unless example + end + + return [:error, "example is not an Example Object"] unless example.is_a?(Hash) + return [:value, example["value"]] if example.key?("value") + # An `externalValue` is a URL. Fetching it would make an offline gate depend on + # the network, which is how a gate learns to skip. + return [:skip, "example is an `externalValue` URL, which this gate does not fetch"] if example.key?("externalValue") + + [:error, "Example Object declares neither `value` nor `externalValue`"] +end + +# --- Load ---------------------------------------------------------------------- + +unless File.file?(OPENAPI_FILE) + warn "ERROR: openapi.json not found at #{OPENAPI_FILE}" + exit 2 +end + +doc = JSON.parse(read_utf8(OPENAPI_FILE)) +components = doc.dig("components", "schemas") || {} + +errors = [] +skipped = [] +counts = Hash.new(0) + +# --- Walk ---------------------------------------------------------------------- + +# Validates one example value against `schema`, tagging every message with +# `where` so a failure names the operation, the example and the field. +validate = lambda do |where, kind, value, schema| + if schema.nil? + skipped << "#{where}: no schema declared alongside the example — nothing to validate against" + return + end + + counts[kind] += 1 + SchemaInstanceValidator.instance_errors("", value, schema, components).each do |msg| + errors << "#{where}: #{msg}" + end +end + +# Records a non-:value example entry. Returns true when the caller should stop. +triaged = lambda do |where, status, payload| + case status + when :error then errors << "#{where}: #{payload}" + when :skip then skipped << "#{where}: #{payload}" + end + status != :value +end + +# Media-type nodes (responses and requestBody bodies). +walk_content = lambda do |where_prefix, kind, content| + (content || {}).each do |media_type, media| + next unless media.is_a?(Hash) + + example_entries(media, doc).each do |name, status, payload| + where = "#{where_prefix}/#{media_type}/examples/#{name}" + next if triaged.call(where, status, payload) + + validate.call(where, kind, payload, media["schema"]) + end + end +end + +# Parameter nodes carry the example beside their own `schema`. +walk_parameters = lambda do |where_prefix, parameters| + (parameters || []).each do |param| + next unless param.is_a?(Hash) + + example_entries(param, doc).each do |name, status, payload| + where = "#{where_prefix} parameters/#{param['name']}/examples/#{name}" + next if triaged.call(where, status, payload) + + validate.call(where, :parameter, payload, param["schema"]) + end + end +end + +(doc["paths"] || {}).each do |path, path_item| + next unless path_item.is_a?(Hash) + + # Path-level parameters are shared by every method, so they are walked once + # per path rather than once per operation — otherwise one example would be + # counted (and any failure reported) once for each method under it. None + # exist in the generated document today; handling them keeps that from + # becoming a silent blind spot if the generator starts hoisting them. + walk_parameters.call(path, path_item["parameters"]) + + path_item.each do |method, op| + next unless HTTP_METHODS.include?(method) && op.is_a?(Hash) + + label = "#{method.upcase} #{path}" + label += " (#{op['operationId']})" if op["operationId"] + + (op["responses"] || {}).each do |code, response| + next unless response.is_a?(Hash) + + walk_content.call("#{label} responses/#{code}", :response, response["content"]) + end + + walk_content.call("#{label} requestBody", :request_body, (op["requestBody"] || {})["content"]) + + walk_parameters.call(label, op["parameters"]) + end +end + +# --- Liveness floor ------------------------------------------------------------ +# +# The seam lives in RESPONSE examples: that is where a projection-added `required` +# entry meets a value Smithy never checked. A run that validates none of them +# passes for the wrong reason — the walk broke, or the examples were deleted. A +# gate that no-ops is the failure mode this gate exists to prevent, so say so +# instead of exiting green. + +if counts[:response].zero? + errors << "no response example was validated — either openapi.json stopped publishing response " \ + "examples or this walk stopped finding them; a vacuous pass is not a pass" +end + +# --- Report -------------------------------------------------------------------- + +total = counts.values.sum + +if errors.empty? + puts "==> Projected examples validate — #{total} checked " \ + "(#{counts[:response]} response, #{counts[:request_body]} request-body, " \ + "#{counts[:parameter]} parameter)" + skipped.sort.each { |s| puts " skipped: #{s}" } + exit 0 +else + warn "Projected-example validation failed:" + errors.sort.each { |e| warn " - #{e}" } + warn "" + warn "These examples are published in openapi.json but contradict the schema they sit under." + warn "A projection-added `required` entry (spec/smithy-build.json `jsonAdd` .../required/-) is not" + warn "checked by `smithy validate`, which only sees the pre-projection Smithy model — so the example" + warn "has to be updated in the same place the requiredness came from. See #638." + exit 1 +end diff --git a/scripts/test-check-projected-examples.rb b/scripts/test-check-projected-examples.rb new file mode 100755 index 0000000000..ebfa0ac162 --- /dev/null +++ b/scripts/test-check-projected-examples.rb @@ -0,0 +1,383 @@ +#!/usr/bin/env ruby +# frozen_string_literal: true + +# Negative-case self-test for scripts/check-projected-examples.rb. +# +# The gate's own `make check` run only exercises the VALID openapi.json, so +# nothing there proves it rejects anything. This test crafts each way a published +# example can contradict its projected schema and asserts the gate rejects it +# (non-zero exit + an expected message fragment), driving the checker through its +# PROJECTED_EXAMPLES_OPENAPI env override against tmp specs. It also confirms the +# real openapi.json still passes (positive control). +# +# Every case mutates a DEEP COPY of the real openapi.json — one mutation each, in +# a tmp file. The tracked spec is never written to. Mutating the real document +# rather than inventing a synthetic one keeps every case anchored to schemas this +# SDK actually ships: case 1 reproduces the #637 defect exactly (`color` in +# `Todolist.required` via `jsonAdd`, absent from both `GetTodolistOrGroup` +# examples) against today's schema. +# +# PROJECTED_EXAMPLES_CHECKER names the checker under test (default +# scripts/check-projected-examples.rb), so a deliberately-mutated copy can be +# driven through the same suite to prove the suite is not vacuous: +# +# mkdir -p /tmp/m/scripts && cp scripts/check-projected-examples.rb \ +# scripts/schema_instance_validator.rb /tmp/m/scripts/ # mutate the COPY +# PROJECTED_EXAMPLES_CHECKER=/tmp/m/scripts/check-projected-examples.rb \ +# ruby scripts/test-check-projected-examples.rb +# +# The copy needs scripts/schema_instance_validator.rb beside it (require_relative). +# Never mutate the tracked file. +# +# Every case below is pinned by at least one guard, and every guard by at least +# one case. These are the MEASURED results of driving mutated copies through this +# suite, not a prediction — each row is one guard removed, and the cases listed +# are exactly the ones that went red (a case goes red either by passing when it +# should fail, or by failing without its expected message): +# +# guard removed in the copy cases that go red +# ---------------------------------------- ------------------------------ +# response-example walk control, 1, 2, 3, 4, 7, 8, 9, +# 11, 12, 13, 14 +# request-body-example walk 5 +# parameter-example walk 6 +# liveness floor (>= 1 response example) 10 +# components/examples `$ref` resolution 11 +# `externalValue` skip 12 +# value-less Example Object rejection 13 +# Example Object type check 14 +# triage keeps :error distinct from :skip 11, 13, 14 +# instance_errors result discarded 1, 2, 3, 4, 5, 6, 7, 8, 9 +# +# The last row is the one that matters most: it unwires the validator itself +# while leaving every walk in place, so a gate that visits all 37 examples and +# concludes nothing still goes red. Coverage and judgement are pinned separately. +# +# Note what is NOT in this table any more. Until #644 the projection published +# response examples still carrying the Smithy output wrapper their schema no +# longer had, and this gate unwrapped them — machinery with its own guards +# (multi-key wrapper, non-object wrapper, empty-wrapper skip) and its own cases. +# #648's `BareResponseExampleMapper` unwraps them in the projection instead, so +# all of that is deleted rather than left dead. Case 7 is what replaced it: put +# the wrapper back and the gate goes red, which makes this the regression guard +# for that mapper. +# +# Run directly (`ruby scripts/test-check-projected-examples.rb`) or via +# `make check-projected-examples` (which runs it after the live check). + +require "json" +require "tmpdir" +require "open3" + +# Per-case lines go to stdout and the failure report to stderr. Unsynced, stdout +# block-buffers when redirected to a file and the report lands ahead of the cases +# it summarizes — which is exactly when someone is reading the log. +$stdout.sync = true + +ROOT = File.expand_path("..", __dir__) +CHECKER = File.expand_path( + ENV.fetch("PROJECTED_EXAMPLES_CHECKER", "scripts/check-projected-examples.rb"), ROOT +) +REAL_OPENAPI = File.join(ROOT, "openapi.json") + +TODOLIST_PATH = "/{accountId}/todolists/{id}" +SUBSCRIPTION_PATH = "/{accountId}/recordings/{recordingId}/subscription.json" +RECORDINGS_PATH = "/{accountId}/projects/recordings.json" + +def read_utf8(path) = File.read(path, encoding: "UTF-8") + +# Run the checker against a given spec; returns [combined_output, status]. +# +# The captured bytes are tagged UTF-8 explicitly. Under LC_ALL=C (which CI uses, +# to prove the readers stay pinned) Open3 hands back a US-ASCII-tagged string, +# and comparing it against a UTF-8 fragment from this file — the messages carry +# em dashes — raises Encoding::CompatibilityError instead of reporting a case. +def run_checker(openapi:) + out, status = Open3.capture2e({ "PROJECTED_EXAMPLES_OPENAPI" => openapi }, "ruby", CHECKER) + [out.dup.force_encoding("UTF-8"), status] +end + +# The real spec, deep-copied, with exactly one mutation applied in the block. +# JSON round-tripping breaks every reference back to the parsed original, so a +# mutation cannot leak into a later case either. +def with_mutated_spec + doc = JSON.parse(read_utf8(REAL_OPENAPI)) + yield doc + Dir.mktmpdir("projected-examples-test") do |dir| + tmp = File.join(dir, "openapi.json") + File.write(tmp, JSON.generate(doc)) + run_checker(openapi: tmp) + end +end + +# Every case anchors on a real operation, so a spec change that moves one out +# from under this suite has to SAY so. Without this the cases would die on +# `NoMethodError: undefined method for nil`, which reads like a bug in the gate +# rather than a test that needs re-anchoring. +def anchor(doc, *path) + node = doc.dig(*path) + raise "self-test anchor #{path.inspect} no longer resolves in openapi.json — " \ + "the spec moved and this case needs re-anchoring, the gate is not at fault" if node.nil? + + node +end + +# The GetTodolistOrGroup 200 response examples map (the shape the #637 defect +# lived in): name => example object, each with a `{"result": {...}}` value. +def todolist_response_examples(doc) + anchor(doc, "paths", TODOLIST_PATH, "get", "responses", "200", "content", "application/json", "examples") +end + +# #648 unwraps the response examples in the projection, so the published value IS +# the Todolist — there is no `result` wrapper left to reach through. +def todolist_payload(doc, example) + todolist_response_examples(doc).fetch(example).fetch("value") +end + +failures = [] + +def expect_pass(failures, label, out, status) + if status.success? + puts " PASS #{label}" + else + puts " FAIL #{label}" + failures << "#{label}: expected PASS but checker failed:\n#{out}" + end +end + +def expect_pass_containing(failures, label, out, status, fragment) + if !status.success? + puts " FAIL #{label}" + failures << "#{label}: expected PASS but checker failed:\n#{out}" + elsif !out.include?(fragment) + puts " FAIL #{label}" + failures << "#{label}: passed but message missing #{fragment.inspect}:\n#{out}" + else + puts " PASS #{label}" + end +end + +def expect_fail(failures, label, out, status, fragment) + if status.success? + puts " FAIL #{label}" + failures << "#{label}: expected FAILURE but checker passed:\n#{out}" + elsif !out.include?(fragment) + puts " FAIL #{label}" + failures << "#{label}: failed as expected but message missing #{fragment.inspect}:\n#{out}" + else + puts " PASS #{label}" + end +end + +puts "==> projected-example self-test (checker: #{CHECKER.sub("#{ROOT}/", '')})" + +# --- Positive control ---------------------------------------------------------- +# +# First, and load-bearing twice over: if the real spec does not pass, every +# negative case below fails for a reason that has nothing to do with its +# mutation. It is also the only thing that pins the bare-response envelope +# unwrap — the real GetTodolistOrGroup examples carry a `result` wrapper the +# projected schema no longer has. + +out, status = run_checker(openapi: REAL_OPENAPI) +expect_pass(failures, "positive control: the real openapi.json passes", out, status) + +# --- 1. The #637 defect, exactly -- THE ONE THAT MATTERS ----------------------- +# +# `Todolist.required` gains `color` through spec/smithy-build.json's +# `jsonAdd .../required/-`. `smithy validate` sees the pre-projection model, +# where `color` is optional, so it passes. Nothing else looked. Both published +# examples shipped without it. + +out, status = with_mutated_spec do |doc| + todolist_payload(doc, "GetTodolistOrGroup_example1").delete("color") + todolist_payload(doc, "GetTodolistOrGroup_example2").delete("color") +end +expect_fail(failures, "1. #637: projection-added required field missing from both examples", out, status, + "GetTodolistOrGroup_example1: missing required field `color`") + +# --- 2. Required field present as null ------------------------------------------ +# +# `comments_app_url` is required and `type: string` with no null union: absence +# and null are different wire facts and neither is legal here. + +out, status = with_mutated_spec do |doc| + todolist_payload(doc, "GetTodolistOrGroup_example1")["comments_app_url"] = nil +end +expect_fail(failures, "2. required field present as null against a non-nullable schema", out, status, + "comments_app_url: required field is null but its schema is not nullable") + +# --- 3. Nested type contradiction ------------------------------------------------ +# +# Inside `creator`, which the example reaches only through a `$ref` to Person — +# so this also pins that the walk descends through refs rather than stopping at +# the top-level object. + +out, status = with_mutated_spec do |doc| + todolist_payload(doc, "GetTodolistOrGroup_example1")["creator"]["id"] = "1" +end +expect_fail(failures, "3. nested type contradiction behind a $ref: creator/id string for integer", out, status, + "creator/id: expected integer, got string") + +# --- 4. Nested missing required -------------------------------------------------- + +out, status = with_mutated_spec do |doc| + todolist_payload(doc, "GetTodolistOrGroup_example2")["creator"].delete("name") +end +expect_fail(failures, "4. nested missing required field: creator/name", out, status, + "missing required field `creator/name`") + +# --- 5. Request-body example ------------------------------------------------------ +# +# Response examples are where the projection seam lives, but request bodies are +# published too and are covered. An element-level fault also pins array descent. + +out, status = with_mutated_spec do |doc| + anchor(doc, "paths", SUBSCRIPTION_PATH, "put", "requestBody", "content", "application/json", + "examples", "UpdateSubscription_example1")["value"]["subscriptions"] = [111, "222"] +end +expect_fail(failures, "5. request-body example: string in an array of integers", out, status, + "subscriptions[1]: expected integer, got string") + +# --- 6. Parameter example ---------------------------------------------------------- +# +# `accountId` is a numeric STRING. A bare integer in the example would document a +# request the SDK cannot send. + +out, status = with_mutated_spec do |doc| + anchor(doc, "paths", RECORDINGS_PATH, "get", "parameters") + .find { |p| p["name"] == "accountId" }["examples"]["ListRecordings_example1"]["value"] = 999 +end +expect_fail(failures, "6. parameter example: integer for a numeric-string parameter", out, status, + "parameters/accountId/examples/ListRecordings_example1: (root): expected string, got integer") + +# --- 7. The wrapper #648's mapper removes must not come back --------------------- +# +# Before #648 the bare-response mappers rewrote the SCHEMA into the bare payload +# and left the wrapper on the example, so every published response example +# contradicted its own schema. `BareResponseExampleMapper` mirrors the unwrapping +# onto examples; this case is the regression guard for it. Re-wrap one example +# and every required field of the bare shape goes missing at once — including +# `color`, the field this whole gate exists for. +# +# This is why the gate does NOT unwrap anything itself. A gate that also accepted +# the wrapped shape would be silent here, and a regression in that mapper would +# ship exactly the way #637 did. + +out, status = with_mutated_spec do |doc| + ex = todolist_response_examples(doc).fetch("GetTodolistOrGroup_example1") + ex["value"] = { "result" => ex["value"] } +end +expect_fail(failures, "7. re-wrapped response example (BareResponseExampleMapper regression)", out, status, + "GetTodolistOrGroup_example1: missing required field `color`") + +# --- 8. A response example that is not the shape its schema declares -------------- +# +# `ListRecordings` publishes an ARRAY of Recording. An object there is the +# category of fault the old wrapper was, stated as what it actually is: a type +# contradiction against the published schema. + +out, status = with_mutated_spec do |doc| + anchor(doc, "paths", RECORDINGS_PATH, "get", "responses", "200", "content", "application/json", + "examples", "ListRecordings_example1")["value"] = { "not" => "an array" } +end +expect_fail(failures, "8. array-schema response example given an object", out, status, + "ListRecordings_example1: (root): expected array, got object") + +# --- 9. Every operation's response examples are reached, not just one ------------- +# +# Four operations publish response examples and #648 gave all ten a real payload. +# A walk that reached only the first would still pass cases 1-4, so fault an +# example on an operation none of those touch. + +out, status = with_mutated_spec do |doc| + anchor(doc, "paths", SUBSCRIPTION_PATH, "put", "responses", "200", "content", "application/json", + "examples", "UpdateSubscription_example2")["value"].delete("count") +end +expect_fail(failures, "9. response example on a fourth operation is reached", out, status, + "UpdateSubscription_example2: missing required field `count`") + +# --- 10. Liveness floor ---------------------------------------------------------------- +# +# Strip every response example in the document. The gate then validates none of +# the class the seam lives in — and would otherwise pass, because request-body +# and parameter examples still count. A gate that no-ops is the failure mode this +# gate exists to prevent. + +out, status = with_mutated_spec do |doc| + doc.fetch("paths").each_value do |path_item| + next unless path_item.is_a?(Hash) + + path_item.each_value do |op| + next unless op.is_a?(Hash) && op["responses"].is_a?(Hash) + + op["responses"].each_value do |response| + next unless response.is_a?(Hash) && response["content"].is_a?(Hash) + + response["content"].each_value { |m| m.delete("examples") if m.is_a?(Hash) } + end + end + end +end +expect_fail(failures, "10. vacuous run: no response example validated", out, status, + "no response example was validated") + +# --- 11. Unresolvable example $ref ------------------------------------------------------- +# +# An example given as a `$ref` into components/examples that does not resolve is +# an unchecked example, not an absent one. + +out, status = with_mutated_spec do |doc| + todolist_response_examples(doc)["GetTodolistOrGroup_example1"] = + { "$ref" => "#/components/examples/NoSuchExample" } +end +expect_fail(failures, "11. example $ref that resolves to nothing", out, status, + "`$ref` does not resolve to an entry in components/examples") + +# --- 12. externalValue ------------------------------------------------------------------- +# +# A remote example is reported as skipped, by name, rather than fetched: an +# offline gate that reaches the network is a gate that skips when the network is +# down. example2 still carries a payload, so the liveness floor is satisfied and +# the run stays green — the point is that the skip is VISIBLE. + +out, status = with_mutated_spec do |doc| + todolist_response_examples(doc)["GetTodolistOrGroup_example1"] = + { "externalValue" => "https://example.invalid/todolist.json" } +end +expect_pass_containing(failures, "12. externalValue example is skipped by name, not fetched", out, status, + "`externalValue` URL, which this gate does not fetch") + +# --- 13. Example Object with no value at all ------------------------------------------------ +# +# Neither `value` nor `externalValue`: there is nothing to check and nothing that +# says so, which is the one thing a skip must never be confused with. + +out, status = with_mutated_spec do |doc| + todolist_response_examples(doc)["GetTodolistOrGroup_example1"] = { "summary" => "no value here" } +end +expect_fail(failures, "13. Example Object declaring neither value nor externalValue", out, status, + "Example Object declares neither `value` nor `externalValue`") + +# --- 14. Not an Example Object at all --------------------------------------------------------- +# +# An `examples` map entry is an Example Object, never a bare value. Accepting a +# bare value here is what would let the singular-`example` reading leak into the +# plural spelling and start unwrapping a payload field named `value`. + +out, status = with_mutated_spec do |doc| + todolist_response_examples(doc)["GetTodolistOrGroup_example1"] = "just a string" +end +expect_fail(failures, "14. examples map entry that is not an Example Object", out, status, + "example is not an Example Object") + +# --- Report -------------------------------------------------------------------- + +if failures.empty? + puts "==> projected-example self-test passed — 1 positive + 14 negative/skip cases" + exit 0 +else + warn "projected-example self-test FAILED:" + failures.each { |f| warn " - #{f}" } + exit 1 +end From 323422e220f78a814594c0a3132cf5a66a3c2277 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Tue, 4 Aug 2026 00:35:54 -0700 Subject: [PATCH 3/5] Run the projected-example gate in `make check` and in CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two places, because membership in one is not membership in the other: the `spec-gates` job ENUMERATES its targets rather than invoking `make check`, so a target added only to the Makefile would run on developer machines and nowhere else — the #580 gap. Joined to `check-targets:` (the list `check:` sub-makes, since #631 wrapped it in a lockfile snapshot) and added as a `spec-gates` step ahead of that job's lockfile diagnostics, which stay last by design. Ruby and openapi.json are its only inputs, so it needs nothing the job does not already have. Run under LC_ALL=C in CI, matching the fixture-coverage job: the reads are pinned to UTF-8, and running the non-UTF-8-locale path in CI is what keeps them that way. --- .github/workflows/test.yml | 15 +++++++++++++++ Makefile | 18 ++++++++++++++++-- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 52e88fa318..86c6ba84b7 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -185,6 +185,21 @@ jobs: - name: Replay decoders cover every live fixture operation run: make check-replay-decoder-parity + # `smithy validate` checks `@examples` against the SMITHY model, where a + # member that `jsonAdd` later appends to a schema's `required` array is + # still natively optional. Nothing compared the projected examples to the + # projected schema, so #637 published two `GetTodolistOrGroup` examples + # that could not satisfy their own contract and a bot reviewer, not CI, + # caught it. The target also runs the gate's self-test, whose first case + # is that defect reproduced exactly. + # + # Under LC_ALL=C so CI exercises the non-UTF-8-locale path (the reads are + # pinned to UTF-8; this proves it stays that way). + - name: Published examples satisfy the schema the projection publishes + run: make check-projected-examples + env: + LC_ALL: C + # Advisory: prints and exits 0. The guarantee is the byte comparison that # runs at the end of every job that installs anything. This is the early # warning the byte comparison cannot give — it names the file and line at diff --git a/Makefile b/Makefile index 289edaf170..cec81ab17f 100644 --- a/Makefile +++ b/Makefile @@ -1036,7 +1036,7 @@ tools: # Spec-shape lints #------------------------------------------------------------------------------ -.PHONY: check-bucket-flat-parity validate-api-gaps check-deprecation-parity kt-check-optional-arrays-and-scalars go-check-optional-pointers test-enhance-request-reachability check-fixture-coverage check-idempotency-parity check-write-semantics-parity check-retry-metadata-parity check-runner-test-reachability check-replay-decoder-parity check-readme-env-vars test-check-readme-env-vars lint-npm-lockfile-writes test-lint-npm-lockfile-writes test-assert-sdk-built test-assert-lockfiles-unchanged +.PHONY: check-bucket-flat-parity validate-api-gaps check-deprecation-parity kt-check-optional-arrays-and-scalars go-check-optional-pointers test-enhance-request-reachability check-fixture-coverage check-idempotency-parity check-write-semantics-parity check-retry-metadata-parity check-runner-test-reachability check-replay-decoder-parity check-readme-env-vars test-check-readme-env-vars lint-npm-lockfile-writes test-lint-npm-lockfile-writes test-assert-sdk-built test-assert-lockfiles-unchanged check-projected-examples # Verify every bucket-scoped GET list operation has a flat-path counterpart # (or is justified in spec/bucket-scoped-allowlist.txt). Cross-project SDK @@ -1068,6 +1068,20 @@ check-fixture-coverage: @./scripts/check-fixture-coverage.sh @ruby ./scripts/test-check-fixture-coverage.rb +# Projected-example guard: every example openapi.json publishes must satisfy +# the schema it sits under IN THE PROJECTION. `smithy validate` checks +# `@examples` against the Smithy model, where a member that `jsonAdd` later +# appends to a schema's `required` array is still natively optional — so a +# projection-added required field can go missing from a published example +# with every other gate green. That shipped in #637 and a bot reviewer, not +# CI, caught it (#638). Reuses scripts/schema_instance_validator.rb, the same +# composition-aware walk check-fixture-coverage uses. The self-test asserts +# the gate rejects each crafted failure mode; the live check above only ever +# exercises the valid spec. +check-projected-examples: + @ruby ./scripts/check-projected-examples.rb + @ruby ./scripts/test-check-projected-examples.rb + # Presence contract for generated Kotlin ARRAY and PRIMITIVE SCALAR properties: # optional -> `T? = null`, required -> `T`, required-and-nullable -> `T?` with no # default, and no zero-value sentinel defaults (`= emptyList()`, `= 0`, `= false`, @@ -1252,7 +1266,7 @@ check: if [ $$rc -ne 0 ]; then exit $$rc; fi; \ echo "==> All checks passed" -check-targets: lint-actions sync-spec-version-check smithy-check smithy-mapper-test behavior-model-check provenance-check sync-api-version-check doc-constants-check url-routes-check bc3-route-parity test-bc3-route-parity go-check-drift go-check-wrapper-drift go-check-generated-drift auth-routable-check kt-check-drift swift-check-drift go-check ts-check rb-check kt-check swift-check py-check conformance check-bucket-flat-parity validate-api-gaps check-deprecation-parity check-fixture-coverage kt-check-optional-arrays-and-scalars go-check-optional-pointers test-enhance-request-reachability check-idempotency-parity check-write-semantics-parity check-retry-metadata-parity check-runner-test-reachability check-replay-decoder-parity check-readme-env-vars test-check-readme-env-vars lint-npm-lockfile-writes test-lint-npm-lockfile-writes test-assert-sdk-built test-assert-lockfiles-unchanged +check-targets: lint-actions sync-spec-version-check smithy-check smithy-mapper-test behavior-model-check provenance-check sync-api-version-check doc-constants-check url-routes-check bc3-route-parity test-bc3-route-parity go-check-drift go-check-wrapper-drift go-check-generated-drift auth-routable-check kt-check-drift swift-check-drift go-check ts-check rb-check kt-check swift-check py-check conformance check-bucket-flat-parity validate-api-gaps check-deprecation-parity check-fixture-coverage kt-check-optional-arrays-and-scalars go-check-optional-pointers test-enhance-request-reachability check-idempotency-parity check-write-semantics-parity check-retry-metadata-parity check-runner-test-reachability check-replay-decoder-parity check-readme-env-vars test-check-readme-env-vars lint-npm-lockfile-writes test-lint-npm-lockfile-writes test-assert-sdk-built test-assert-lockfiles-unchanged check-projected-examples @: # Clean all build artifacts From 24de55b9fe29fb131e21aea85e6b337c1fe97d82 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Tue, 4 Aug 2026 01:07:52 -0700 Subject: [PATCH 4/5] Judge a root null instead of exempting it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `instance_errors` returned no errors for any null value. For a NESTED null that is correct and deliberate: the Smithy-derived OpenAPI under-marks some nullable optionals, and something above has already looked — a required-but-null field is caught by the required loop in its parent, a null element by the items check in its array. The exemption means "the enclosing context already judged this". A root value has no enclosing context, so nothing was standing behind the exemption. A published example of `value: null` under a non-nullable schema returned clean AND was counted as validated — the projected-example gate approving exactly the class of contradiction it was built to catch. Measured before the fix, against a crafted spec: 37 checked, exit 0, with a null `GetTodolistOrGroup` response example and a null `accountId` parameter example both waved through. Root nulls are now checked against the schema's nullability. Two self-test cases pin the guard and a third pins its over-correction: rejecting every root null would pass both negative cases and then start failing legitimate examples for required-and-nullable shapes — the class `Todolist.color` belongs to — so case 16 asserts a null IS accepted where the schema permits one. check-fixture-coverage shares this validator and is unaffected: it rejects a null root before calling in, so the new branch is unreachable from there. Its own self-test (1 positive + 8 negative + 20 synthetic) still passes. --- scripts/schema_instance_validator.rb | 30 +++++++++-- scripts/test-check-projected-examples.rb | 65 +++++++++++++++++++++--- 2 files changed, 83 insertions(+), 12 deletions(-) diff --git a/scripts/schema_instance_validator.rb b/scripts/schema_instance_validator.rb index 5d703cbd56..27ca7f2289 100644 --- a/scripts/schema_instance_validator.rb +++ b/scripts/schema_instance_validator.rb @@ -194,13 +194,33 @@ def merged_constraints(schema, components, visited = Set.new, depth = 0) # Composition-aware validation of `value` against `schema`. Reports # (path-tagged) errors for a missing required field, a required field present as # null against a non-nullable schema, a null array element against a non-nullable - # item schema, and a present value whose JSON type contradicts the declared type. - # Optional object-field nulls are tolerated: the Smithy-derived OpenAPI - # under-marks some nullable optionals (e.g. Person.bio/location are `type:string` - # yet the wire sends null), so flagging them would be a false positive. + # item schema, a ROOT null against a non-nullable schema, and a present value + # whose JSON type contradicts the declared type. + # + # NESTED nulls are tolerated: the Smithy-derived OpenAPI under-marks some + # nullable optionals (e.g. Person.bio/location are `type:string` yet the wire + # sends null), so flagging them would be a false positive. That exemption is + # only sound because something above HAS looked — a required-but-null field is + # caught by the required loop in its parent, a null array element by the items + # check in its array. It is an "the enclosing context already judged this" + # rule, not an "any null is fine" rule. + # + # A ROOT null (depth 0) has no enclosing context and therefore nothing standing + # behind the exemption, so it is checked here. Skipping it let a published + # example of `value: null` under a non-nullable schema be counted as VALIDATED + # — the gate approving exactly the class of contradiction it exists to catch. def instance_errors(prefix, value, schema, components, depth = 0) return [] if depth > 60 - return [] if value.nil? # optional-null tolerated; required-/element-null handled in context + + if value.nil? + return [] unless depth.zero? + + _, _, _, root_nullable, = merged_constraints(schema, components) + return [] if root_nullable + + label = prefix.empty? ? "(root)" : prefix + return ["#{label}: value is null but the schema is not nullable"] + end req, props, _types, _nullable, items, alt_groups, type_sets = merged_constraints(schema, components) diff --git a/scripts/test-check-projected-examples.rb b/scripts/test-check-projected-examples.rb index ebfa0ac162..6b13e87445 100755 --- a/scripts/test-check-projected-examples.rb +++ b/scripts/test-check-projected-examples.rb @@ -38,20 +38,33 @@ # guard removed in the copy cases that go red # ---------------------------------------- ------------------------------ # response-example walk control, 1, 2, 3, 4, 7, 8, 9, -# 11, 12, 13, 14 +# 11, 12, 13, 14, 15a, 16 # request-body-example walk 5 -# parameter-example walk 6 +# parameter-example walk 6, 15b # liveness floor (>= 1 response example) 10 # components/examples `$ref` resolution 11 # `externalValue` skip 12 # value-less Example Object rejection 13 # Example Object type check 14 # triage keeps :error distinct from :skip 11, 13, 14 -# instance_errors result discarded 1, 2, 3, 4, 5, 6, 7, 8, 9 +# instance_errors result discarded 1, 2, 3, 4, 5, 6, 7, 8, 9, +# 15a, 15b +# root-null check (schema_instance_validator) 15a, 15b +# ...and its nullability test, i.e. a version +# that bans EVERY root null 16 # -# The last row is the one that matters most: it unwires the validator itself -# while leaving every walk in place, so a gate that visits all 37 examples and -# concludes nothing still goes red. Coverage and judgement are pinned separately. +# `instance_errors result discarded` is the row that matters most: it unwires the +# validator itself while leaving every walk in place, so a gate that visits all +# 37 examples and concludes nothing still goes red. Coverage and judgement are +# pinned separately. +# +# The last two rows are one guard and its over-correction, pinned in both +# directions on purpose. Root nulls were the third hole a reviewer found in this +# gate, and all three were the same shape: A PATH THAT RETURNS SUCCESS WITHOUT +# EXAMINING THE THING. The cheapest way to close it — reject every root null — +# would pass 15a/15b and start rejecting legitimate examples for +# required-and-nullable shapes, which is the class `Todolist.color` belongs to. +# So 16 pins that the check consults nullability rather than banning nulls. # # Note what is NOT in this table any more. Until #644 the projection published # response examples still carrying the Smithy output wrapper their schema no @@ -371,10 +384,48 @@ def expect_fail(failures, label, out, status, fragment) expect_fail(failures, "14. examples map entry that is not an Example Object", out, status, "example is not an Example Object") +# --- 15. Root null against a non-nullable schema --------------------------------- +# +# `instance_errors` tolerates a null because the ENCLOSING context has already +# judged it — a required-but-null field is caught by the required loop in its +# parent, a null element by the items check in its array. A root value has no +# enclosing context, so before the fix this returned no errors and the example +# was COUNTED AS VALIDATED: the gate approving exactly the contradiction it +# exists to catch. Both a response and a parameter example, because the early +# return sat below every kind of caller alike. + +out, status = with_mutated_spec do |doc| + todolist_response_examples(doc).fetch("GetTodolistOrGroup_example1")["value"] = nil +end +expect_fail(failures, "15a. response example value:null against a non-nullable schema", out, status, + "GetTodolistOrGroup_example1: (root): value is null but the schema is not nullable") + +out, status = with_mutated_spec do |doc| + anchor(doc, "paths", RECORDINGS_PATH, "get", "parameters") + .find { |p| p["name"] == "accountId" }["examples"]["ListRecordings_example1"]["value"] = nil +end +expect_fail(failures, "15b. parameter example value:null against a non-nullable schema", out, status, + "parameters/accountId/examples/ListRecordings_example1: (root): value is null but the schema is not nullable") + +# --- 16. Root null against a NULLABLE schema is still fine ------------------------- +# +# The guard for case 15 has to check nullability, not ban root nulls. Without +# this case the cheapest way to pass 15 — reject every null — would look correct +# and would start failing legitimate examples the moment the spec publishes one +# for a required-and-nullable shape, which is the exact class `Todolist.color` +# belongs to. + +out, status = with_mutated_spec do |doc| + param = anchor(doc, "paths", RECORDINGS_PATH, "get", "parameters").find { |p| p["name"] == "accountId" } + param["schema"] = { "type" => %w[string null] } + param["examples"]["ListRecordings_example1"]["value"] = nil +end +expect_pass(failures, "16. root null IS allowed where the schema permits null", out, status) + # --- Report -------------------------------------------------------------------- if failures.empty? - puts "==> projected-example self-test passed — 1 positive + 14 negative/skip cases" + puts "==> projected-example self-test passed — 2 positive + 16 negative/skip cases" exit 0 else warn "projected-example self-test FAILED:" From e27387de831e1c1bddd4fced7bed52ecef50cdc4 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Tue, 4 Aug 2026 11:32:12 -0700 Subject: [PATCH 5/5] Treat an unresolvable schema $ref as an error, not an unconstrained schema merged_constraints resolved a $ref as components[name] and absorbed the result without checking it. A miss handed it nil, which takes the non-Hash early return: no required fields, no type constraints, no items, and nullable true. Absorbed into the enclosing conjunction that does not read as "unknown", it reads as "unconstrained", so every example sitting under a broken pointer validated -- root nulls included -- and the run counted them among the checked and exited 0. Point the GetTodolistOrGroup 200 response schema at a component that does not exist, delete `color` from one example and null the other, and before this change the gate reported "37 checked" and exit 0 while validating neither. Those are the two defects it was built to catch, #637 and the root null, swallowed by a typo in a pointer. Resolution now raises UnresolvableRef, naming the ref: when it is not a components/schemas pointer at all, when the component is absent, and when it is present but not a schema object. instance_errors converts it into an ordinary path-tagged finding, so both callers report it beside every other error and exit non-zero, and the message carries the most specific path the innermost frame had. A $ref back to an already-visited component is still a cycle rather than a failure. The generated openapi.json has 422 distinct refs, all of them resolvable components/schemas pointers to schema objects, so this is strictly a guard: check-fixture-coverage, the other caller, is unaffected. Self-test case 17a is the reproduction, deliberately composite -- the bad pointer plus the missing `color` plus the root null -- so removing the check makes it go red for the reason that matters: the gate finds nothing whatsoever to say about two examples that contradict their schema twice over. 17b covers a ref resolving to a non-object. --- scripts/schema_instance_validator.rb | 64 +++++++++++++++++++++--- scripts/test-check-projected-examples.rb | 63 +++++++++++++++++++++-- 2 files changed, 114 insertions(+), 13 deletions(-) diff --git a/scripts/schema_instance_validator.rb b/scripts/schema_instance_validator.rb index 27ca7f2289..5efa04b1f6 100644 --- a/scripts/schema_instance_validator.rb +++ b/scripts/schema_instance_validator.rb @@ -30,6 +30,10 @@ # * anyOf/oneOf as AT-LEAST-ONE (the value must satisfy some branch; a group is # nullable only if a branch is) # +# Partial is not the same as permissive. A `$ref` this validator cannot resolve +# is an ERROR, never an absent constraint — see `UnresolvableRef`. "I did not +# check this" and "this checks out" must not share an exit code. +# # Intentionally NOT implemented (out of scope unless a case is reached by the # actual generated schemas): exact-one `oneOf` selection, `enum`, `const`, # discriminators, `pattern`, `format`, numeric bounds, `additionalProperties`, @@ -41,6 +45,17 @@ # receiverless call style the extraction moved out of; `SchemaInstanceValidator.x` # works too. module SchemaInstanceValidator + # Raised when a schema-position `$ref` cannot be resolved to a component schema + # object. Resolution failure is FATAL rather than "this node adds no + # constraints": see the resolution block in `merged_constraints`. + # + # `instance_errors` converts it into an ordinary path-tagged validation error, + # so both gates report it beside every other finding and exit non-zero. A + # caller that reaches `merged_constraints` directly gets the raise, which is + # also correct — the validator cannot answer a question about a schema it + # cannot read. + class UnresolvableRef < StandardError; end + module_function # Ruby name of a `$ref`, or nil. @@ -155,11 +170,38 @@ def merged_constraints(schema, components, visited = Set.new, depth = 0) alt_groups.concat(a2) end - name = ref_name(schema) - if name && !visited.include?(name) - visited << name - absorb.call(components[name]) - # fall through to local keywords (OpenAPI 3.1 permits $ref siblings) + # `$ref` resolution is fatal on failure, not silent. + # + # This used to be `absorb.call(components[name])` with no check, and a nil + # (or otherwise non-Hash) target takes the `!schema.is_a?(Hash)` return + # above: no required fields, no type constraints, no items, and + # `nullable = true`. Absorbed into the enclosing conjunction that is not + # "unknown", it is "unconstrained" — EVERY value validates against a broken + # pointer, root nulls included, and the gate counts the example as checked + # and exits 0. A validator that reports success for a schema it never read is + # the failure mode both callers exist to prevent, so say the pointer is + # broken instead of agreeing with whatever it points at. + # + # A `$ref` that names an already-`visited` component is a cycle, not a + # failure — it resolved once on the way in and the guard stops the recursion. + if schema["$ref"].is_a?(String) + ref = schema["$ref"] + name = ref_name(schema) + raise UnresolvableRef, "unresolvable `$ref` `#{ref}`: not a `#/components/schemas/` pointer" unless name + + unless visited.include?(name) + target = components[name] + raise UnresolvableRef, "unresolvable `$ref` `#{ref}`: no such entry in components/schemas" if target.nil? + + unless target.is_a?(Hash) + raise UnresolvableRef, + "unresolvable `$ref` `#{ref}`: components/schemas/#{name} is #{target.class}, not a schema object" + end + + visited << name + absorb.call(target) + # fall through to local keywords (OpenAPI 3.1 permits $ref siblings) + end end t, nn = allowed_types(schema) @@ -209,16 +251,22 @@ def merged_constraints(schema, components, visited = Set.new, depth = 0) # behind the exemption, so it is checked here. Skipping it let a published # example of `value: null` under a non-nullable schema be counted as VALIDATED # — the gate approving exactly the class of contradiction it exists to catch. + # + # An unresolvable `$ref` anywhere in the schema is reported here as a finding + # rather than propagating: the rescue sits at every recursion level, so the + # innermost frame that touched the broken pointer names it with the most + # specific path it has. def instance_errors(prefix, value, schema, components, depth = 0) return [] if depth > 60 + label = prefix.empty? ? "(root)" : prefix + if value.nil? return [] unless depth.zero? _, _, _, root_nullable, = merged_constraints(schema, components) return [] if root_nullable - label = prefix.empty? ? "(root)" : prefix return ["#{label}: value is null but the schema is not nullable"] end @@ -230,7 +278,6 @@ def instance_errors(prefix, value, schema, components, depth = 0) type_sets.each do |ts| next if type_matches?(ts, value) - label = prefix.empty? ? "(root)" : prefix return ["#{label}: expected #{ts.to_a.sort.join('|')}, got #{json_type(value)}"] end @@ -242,7 +289,6 @@ def instance_errors(prefix, value, schema, components, depth = 0) alt_groups.each do |branches| next if branches.any? { |branch| instance_errors(prefix, value, branch, components, depth + 1).empty? } - label = prefix.empty? ? "(root)" : prefix errs << "#{label}: value matches none of the #{branches.length} allowed alternatives (anyOf/oneOf)" end @@ -275,5 +321,7 @@ def instance_errors(prefix, value, schema, components, depth = 0) end errs + rescue UnresolvableRef => e + ["#{label}: #{e.message}"] end end diff --git a/scripts/test-check-projected-examples.rb b/scripts/test-check-projected-examples.rb index 6b13e87445..f119575478 100755 --- a/scripts/test-check-projected-examples.rb +++ b/scripts/test-check-projected-examples.rb @@ -38,7 +38,8 @@ # guard removed in the copy cases that go red # ---------------------------------------- ------------------------------ # response-example walk control, 1, 2, 3, 4, 7, 8, 9, -# 11, 12, 13, 14, 15a, 16 +# 11, 12, 13, 14, 15a, 16, 17a, +# 17b # request-body-example walk 5 # parameter-example walk 6, 15b # liveness floor (>= 1 response example) 10 @@ -48,24 +49,32 @@ # Example Object type check 14 # triage keeps :error distinct from :skip 11, 13, 14 # instance_errors result discarded 1, 2, 3, 4, 5, 6, 7, 8, 9, -# 15a, 15b +# 15a, 15b, 17a, 17b # root-null check (schema_instance_validator) 15a, 15b # ...and its nullability test, i.e. a version # that bans EVERY root null 16 +# schema `$ref` resolution check +# (schema_instance_validator) 17a, 17b # # `instance_errors result discarded` is the row that matters most: it unwires the # validator itself while leaving every walk in place, so a gate that visits all # 37 examples and concludes nothing still goes red. Coverage and judgement are # pinned separately. # -# The last two rows are one guard and its over-correction, pinned in both +# The root-null rows are one guard and its over-correction, pinned in both # directions on purpose. Root nulls were the third hole a reviewer found in this -# gate, and all three were the same shape: A PATH THAT RETURNS SUCCESS WITHOUT +# gate, and all four were the same shape: A PATH THAT RETURNS SUCCESS WITHOUT # EXAMINING THE THING. The cheapest way to close it — reject every root null — # would pass 15a/15b and start rejecting legitimate examples for # required-and-nullable shapes, which is the class `Todolist.color` belongs to. # So 16 pins that the check consults nullability rather than banning nulls. # +# The last row is the fourth hole, and the widest: an unresolvable schema `$ref` +# used to yield an UNCONSTRAINED schema rather than an error, so every example +# under a broken pointer passed. See case 17. Every one of these four was found +# by adversarial review, and none by this self-test — which is the argument for +# keeping the table honest rather than decorative. +# # Note what is NOT in this table any more. Until #644 the projection published # response examples still carrying the Smithy output wrapper their schema no # longer had, and this gate unwrapped them — machinery with its own guards @@ -422,10 +431,54 @@ def expect_fail(failures, label, out, status, fragment) end expect_pass(failures, "16. root null IS allowed where the schema permits null", out, status) +# --- 17. A `$ref` the validator cannot resolve ----------------------------------- +# +# The fourth hole, and the same shape as the other three: A PATH THAT RETURNS +# SUCCESS WITHOUT EXAMINING THE THING — this one wide enough to swallow the whole +# gate. `merged_constraints` resolved a `$ref` as `components[name]` and absorbed +# the result unchecked; a miss handed it nil, which takes the non-Hash return — +# no required fields, no type constraints, and NULLABLE. Not "unknown": +# "unconstrained". So every example under a broken pointer validated, root nulls +# included, and the run reported them among the checked. +# +# 17a is the adversarial-review reproduction, unchanged, and it is deliberately a +# composite: the bad pointer PLUS the two defects this gate exists to catch — +# case 1's missing `color` (#637) and case 15a's root null. Measured against the +# pre-fix validator, all three together produced: +# +# ==> Projected examples validate — 37 checked (10 response, 5 request-body, +# 22 parameter) +# exit 0 +# +# which is why one case carries all three. Drop the resolution check and 17a goes +# red for the only reason that matters — not because the message changed, but +# because the checker finds NOTHING WHATSOEVER to say about two examples that +# contradict their schema twice over. + +out, status = with_mutated_spec do |doc| + anchor(doc, "paths", TODOLIST_PATH, "get", "responses", "200", "content", "application/json")["schema"] = + { "$ref" => "#/components/schemas/NoSuchSchema" } + todolist_payload(doc, "GetTodolistOrGroup_example1").delete("color") + todolist_response_examples(doc).fetch("GetTodolistOrGroup_example2")["value"] = nil +end +expect_fail(failures, "17a. unresolvable $ref is an error, not an unconstrained schema", out, status, + "unresolvable `$ref` `#/components/schemas/NoSuchSchema`: no such entry in components/schemas") + +# 17b. Present but not a schema object. Same class, same silent outcome before +# the fix — the non-Hash return does not care WHY the target was not a Hash. + +out, status = with_mutated_spec do |doc| + doc["components"]["schemas"]["NotASchema"] = "this is a string, not a schema object" + anchor(doc, "paths", TODOLIST_PATH, "get", "responses", "200", "content", "application/json")["schema"] = + { "$ref" => "#/components/schemas/NotASchema" } +end +expect_fail(failures, "17b. $ref resolving to a non-object is an error", out, status, + "components/schemas/NotASchema is String, not a schema object") + # --- Report -------------------------------------------------------------------- if failures.empty? - puts "==> projected-example self-test passed — 2 positive + 16 negative/skip cases" + puts "==> projected-example self-test passed — 2 positive + 18 negative/skip cases" exit 0 else warn "projected-example self-test FAILED:"