From b7d77b708d3bcbe234b1262478bb8a9068487eb6 Mon Sep 17 00:00:00 2001 From: Meta Vens Date: Tue, 22 Sep 2026 21:12:23 +0200 Subject: [PATCH] Fix batch 4 (R-X, S-4/S-10): the error schema matches the wire, and the validator refuses a keyword it does not implement CultNetErrorMessage is a MessagePack-CSharp map-mode object, so every declared key is written on the wire, nil included - the same fact R-S already established for the raw document header. cultnet.error.schema.json was missing routingHint entirely and typed code/details as always-present non-null values; Soul's P7 found real bytes failing it on exactly this. routingHint, code and details are now `required`, with nullable types matching CultNetErrorMessage/CultNetErrorDetails. A new test decodes real bytes for all four refusal codes (selection_invalid, cursor_stale, cursor_invalid, reference_outside_target) against the schema. MiniJsonSchemaValidator: - implements minimum, maximum, maxLength and maxItems, which were declared in committed schemas (fieldPredicate's number pattern, selection.limit, ...) but silently never checked; - anchors `pattern` to a full match (Index==0 && Length==value.Length) instead of Regex.IsMatch's "search anywhere", so .NET's `$` (which, unlike ECMA-262's, matches immediately before one trailing newline even without RegexOptions.Multiline) can no longer let e.g. "5\n" pass a schema author wrote as "exactly one digit"; - refuses a schema node that uses a keyword this validator gives no constraint semantics to, instead of silently ignoring it. Running every committed contracts/cultnet/*.schema.json schema through this found 12 such uses, none reached by any existing test: `allOf`/`if`/`then`/`else` in cultnet.database-change-raw.schema.json, and `uniqueItems` (six places) plus one `anyOf` in the cultmesh.verse-catalog-*.schema.json pair. contentEncoding, $schema/$id/$defs/title/description/default and the x-cult-runtime-type vendor extension are treated as metadata with no constraint semantics to implement, not refused. Co-Authored-By: Claude Sonnet 5 --- contracts/cultnet/cultnet.error.schema.json | 33 +++++--- .../CultNetSchemaContractTests.cs | 39 +++++++++ .../MiniJsonSchemaValidator.cs | 79 +++++++++++++++++-- 3 files changed, 135 insertions(+), 16 deletions(-) diff --git a/contracts/cultnet/cultnet.error.schema.json b/contracts/cultnet/cultnet.error.schema.json index e4277f3c..8653cbd5 100644 --- a/contracts/cultnet/cultnet.error.schema.json +++ b/contracts/cultnet/cultnet.error.schema.json @@ -2,29 +2,44 @@ "$schema": "https://json-schema.org/draft/2020-12/schema", "$id": "https://github.com/GameCult/CultLib/contracts/cultnet/cultnet.error.schema.json", "title": "CultNet Error Message", + "description": "R-X (docs/cultnet-selection-cut.md, fix batch 4): CultNetErrorMessage is a MessagePack-CSharp map-mode object, so every declared key is written on the wire, nil included - routingHint, code and details are `required` here the same as cultnet.raw-document-header.schema.json's nullable fields, not optional just because a given refusal leaves them null.", "type": "object", "required": [ "schemaVersion", - "error" + "error", + "routingHint", + "code", + "details" ], "additionalProperties": false, "properties": { "schemaVersion": { "const": "cultnet.error.v0" }, "error": { "type": "string", "minLength": 1 }, + "routingHint": { + "description": "Optional shard routing detail for an authority failure. Null for every selection/cursor refusal code (R-N).", + "type": ["object", "null"], + "additionalProperties": false, + "required": ["shard", "reason"], + "properties": { + "shard": { "type": ["object", "null"] }, + "reason": { "type": "string" } + } + }, "code": { "description": "Machine-readable refusal code (R-N). Additive: a v0 peer that does not know this field ignores it.", - "type": "string", - "enum": ["selection_invalid", "cursor_stale", "cursor_invalid", "reference_outside_target"] + "type": ["string", "null"], + "enum": [null, "selection_invalid", "cursor_stale", "cursor_invalid", "reference_outside_target"] }, "details": { - "description": "Code-specific structured detail (R-N). Every field is populated only for the code that defines it.", - "type": "object", + "description": "Code-specific structured detail (R-N). Every field is populated only for the code that defines it; the others are null, not omitted.", + "type": ["object", "null"], "additionalProperties": false, + "required": ["field", "value", "asOf", "current"], "properties": { - "field": { "description": "selection_invalid: the selection field that failed.", "type": "string" }, - "value": { "description": "selection_invalid: the offending value, when there is one string worth naming.", "type": "string" }, - "asOf": { "description": "cursor_stale: the cursor's own minted asOf.", "type": "integer", "minimum": 0 }, - "current": { "description": "cursor_stale: the answering server's current asOf.", "type": "integer", "minimum": 0 } + "field": { "description": "selection_invalid: the selection field that failed.", "type": ["string", "null"] }, + "value": { "description": "selection_invalid: the offending value, when there is one string worth naming.", "type": ["string", "null"] }, + "asOf": { "description": "cursor_stale: the cursor's own minted asOf.", "type": ["integer", "null"], "minimum": 0 }, + "current": { "description": "cursor_stale: the answering server's current asOf.", "type": ["integer", "null"], "minimum": 0 } } } } diff --git a/tests/GameCult.Networking.Tests/CultNetSchemaContractTests.cs b/tests/GameCult.Networking.Tests/CultNetSchemaContractTests.cs index 8e3d60a4..b5f4f08f 100644 --- a/tests/GameCult.Networking.Tests/CultNetSchemaContractTests.cs +++ b/tests/GameCult.Networking.Tests/CultNetSchemaContractTests.cs @@ -161,5 +161,44 @@ namespace GameCult.Networking.Tests Assert.That(errors, Is.Empty, string.Join("; ", errors)); } + + // R-X (docs/cultnet-selection-cut.md, fix batch 4): CultNetErrorMessage is a MessagePack-CSharp + // map-mode object - routingHint/code/details are always written, nil included, the same as any + // other declared key. Soul's P7 found the schema disagreed with the real bytes on exactly this: + // routingHint undeclared, additionalProperties:false, and details.asOf/current typed as a plain + // integer instead of ["integer","null"]. This decodes real bytes for all four refusal codes. + [Test] + public void ErrorMessage_SelectionInvalidRealBytes_MatchesItsSchema() + { + var ex = new CultNetSelectionInvalidException("schemas", null, "selection.schemas must be non-empty."); + AssertErrorMatchesSchema(CultNetErrorMessage.ForSelectionInvalid(ex)); + } + + [Test] + public void ErrorMessage_CursorStaleRealBytes_MatchesItsSchema() + { + var ex = new CultNetSelectionCursorException("cursor_stale", "stale", asOf: 3, current: 7); + AssertErrorMatchesSchema(CultNetErrorMessage.ForCursor(ex)); + } + + [Test] + public void ErrorMessage_CursorInvalidRealBytes_MatchesItsSchema() + { + var ex = new CultNetSelectionCursorException("cursor_invalid", "The cursor does not decode."); + AssertErrorMatchesSchema(CultNetErrorMessage.ForCursor(ex)); + } + + [Test] + public void ErrorMessage_ReferenceOutsideTargetRealBytes_MatchesItsSchema() + { + var ex = new CultNetSelectionReferenceOutsideTargetException("citer-schema", "citer-key", "Design", "outside-schema", "outside-key"); + AssertErrorMatchesSchema(CultNetErrorMessage.ForReferenceOutsideTarget(ex)); + } + + private static void AssertErrorMatchesSchema(CultNetErrorMessage message) + { + var validator = new MiniJsonSchemaValidator(SchemaDir()); + validator.AssertValid(ToJson(message), "cultnet.error.schema.json"); + } } } diff --git a/tests/GameCult.Networking.Tests/MiniJsonSchemaValidator.cs b/tests/GameCult.Networking.Tests/MiniJsonSchemaValidator.cs index 61a0a02c..74a50465 100644 --- a/tests/GameCult.Networking.Tests/MiniJsonSchemaValidator.cs +++ b/tests/GameCult.Networking.Tests/MiniJsonSchemaValidator.cs @@ -12,14 +12,39 @@ namespace GameCult.Networking.Tests /// A deliberately small JSON Schema (draft 2020-12) validator covering only the constructs /// contracts/cultnet/*.schema.json actually use: object/array/string/integer/boolean/null types /// (plain or as a ["type","null"] union), required, additionalProperties: false, properties, - /// items, const, enum, pattern, minLength, minItems, oneOf, not, and $ref - same-document - /// ("#/$defs/name"), cross-document ("other.schema.json"), and both ("other.schema.json#/$defs/name"). - /// Not a general validator: it exists for R-S (docs/cultnet-selection-cut.md, fix batch 3) to - /// decode real C# wire bytes and check them against the committed schema files, which no - /// dependency in this repo already does - see CultNetSchemaContractTests. + /// items, const, enum, pattern, minLength, maxLength, minItems, maxItems, minimum, maximum, oneOf, + /// not, and $ref - same-document ("#/$defs/name"), cross-document ("other.schema.json"), and both + /// ("other.schema.json#/$defs/name"). Not a general validator: it exists for R-S + /// (docs/cultnet-selection-cut.md, fix batch 3) to decode real C# wire bytes and check them against + /// the committed schema files, which no dependency in this repo already does - see + /// CultNetSchemaContractTests. + /// R-X (fix batch 4): a schema keyword this validator does not implement is refused, not silently + /// ignored - see / and + /// . Before this, a schema using e.g. allOf/anyOf/if-then-else/ + /// uniqueItems validated an instance as if that keyword were not there at all, which is silent + /// under-validation, not the "no such constraint" the schema author intended. /// internal sealed class MiniJsonSchemaValidator { + // Keywords this validator gives real constraint semantics to. + private static readonly HashSet ImplementedKeywords = new(StringComparer.Ordinal) + { + "$ref", "oneOf", "not", "const", "enum", "type", + "minLength", "maxLength", "pattern", "minItems", "maxItems", "minimum", "maximum", + "items", "required", "properties", "additionalProperties" + }; + + // Keywords that are pure annotation/metadata - present on many schema nodes, never checked + // against an instance by any JSON Schema vocabulary this validator implements, so there is + // nothing for CheckSupportedKeywords to refuse. contentEncoding/contentMediaType/contentSchema + // are "media" annotations that draft 2020-12 itself does not assign validation behavior to + // without an explicit vocabulary; x-cult-runtime-type is this repo's own vendor extension. + private static readonly HashSet MetadataKeywords = new(StringComparer.Ordinal) + { + "$schema", "$id", "$defs", "title", "description", "default", + "contentEncoding", "x-cult-runtime-type" + }; + private readonly string _schemaDir; private readonly Dictionary _docCache = new(StringComparer.Ordinal); @@ -89,6 +114,8 @@ namespace GameCult.Networking.Tests private void ValidateAgainst(JsonElement instance, JsonElement schema, string file, string path, List errors) { + CheckSupportedKeywords(schema, path, errors); + if (schema.TryGetProperty("$ref", out var refProp)) { var (resolved, resolvedFile) = Resolve(refProp.GetString()!, file); @@ -155,8 +182,26 @@ namespace GameCult.Networking.Tests var value = instance.GetString()!; if (schema.TryGetProperty("minLength", out var minLength) && value.Length < minLength.GetInt32()) errors.Add($"{path}: string shorter than minLength {minLength.GetInt32()}"); - if (schema.TryGetProperty("pattern", out var pattern) && !Regex.IsMatch(value, pattern.GetString()!)) - errors.Add($"{path}: \"{value}\" does not match pattern {pattern.GetString()}"); + if (schema.TryGetProperty("maxLength", out var maxLength) && value.Length > maxLength.GetInt32()) + errors.Add($"{path}: string longer than maxLength {maxLength.GetInt32()}"); + if (schema.TryGetProperty("pattern", out var pattern)) + { + // R-X: a full match, not Regex.IsMatch's "search anywhere" - and not .NET's own `$`, + // which (unlike ECMA-262's) matches immediately before a single trailing newline even + // without RegexOptions.Multiline, so an author-anchored "^...$" pattern here would + // wrongly accept e.g. "5\n" for a pattern that means "exactly one digit". + var match = Regex.Match(value, pattern.GetString()!); + if (!match.Success || match.Index != 0 || match.Length != value.Length) + errors.Add($"{path}: \"{value}\" does not match pattern {pattern.GetString()}"); + } + } + + if (instance.ValueKind == JsonValueKind.Number) + { + if (schema.TryGetProperty("minimum", out var minimum) && instance.GetDouble() < minimum.GetDouble()) + errors.Add($"{path}: {instance} is less than minimum {minimum}"); + if (schema.TryGetProperty("maximum", out var maximum) && instance.GetDouble() > maximum.GetDouble()) + errors.Add($"{path}: {instance} is greater than maximum {maximum}"); } if (instance.ValueKind == JsonValueKind.Array) @@ -164,6 +209,8 @@ namespace GameCult.Networking.Tests var items = instance.EnumerateArray().ToArray(); if (schema.TryGetProperty("minItems", out var minItems) && items.Length < minItems.GetInt32()) errors.Add($"{path}: array shorter than minItems {minItems.GetInt32()}"); + if (schema.TryGetProperty("maxItems", out var maxItems) && items.Length > maxItems.GetInt32()) + errors.Add($"{path}: array longer than maxItems {maxItems.GetInt32()}"); if (schema.TryGetProperty("items", out var itemSchema)) { for (var i = 0; i < items.Length; i++) @@ -207,6 +254,24 @@ namespace GameCult.Networking.Tests } } + /// + /// R-X: refuses a schema node that uses a keyword this validator gives no behaviour to - allOf, + /// anyOf, if/then/else and uniqueItems are the ones found in the committed contracts at the time + /// this check was added. Before this, such a keyword was silently invisible: an instance that + /// violated it still validated clean, because nothing ever looked at the keyword to begin with. + /// Only checks the schema node actually reached during this validation (the same nodes + /// ValidateAgainst recurses into) - it is not a whole-document schema linter. + /// + private static void CheckSupportedKeywords(JsonElement schema, string path, List errors) + { + if (schema.ValueKind != JsonValueKind.Object) return; + foreach (var property in schema.EnumerateObject()) + { + if (!ImplementedKeywords.Contains(property.Name) && !MetadataKeywords.Contains(property.Name)) + errors.Add($"{path}: schema uses keyword \"{property.Name}\", which this validator does not implement"); + } + } + private static bool MatchesType(JsonElement instance, string type) => type switch { "object" => instance.ValueKind == JsonValueKind.Object,