From 66b5e60b4360ae54cb8f93d1d79355844fbe338d Mon Sep 17 00:00:00 2001 From: Meta Vens Date: Tue, 22 Sep 2026 19:44:18 +0200 Subject: [PATCH] Fix batch 3: R-N wire refusal codes, R-P one authorizer id, R-T removal/cites gaps R-N: cultnet.error.v0 gains additive code/details fields (contracts/cultnet). CultNetErrorMessage grows ForSelectionInvalid/ForCursor/ForReferenceOutsideTarget factories; both C# servers (CultNetDatabaseServer for snapshot+live, CultNetDatabaseSubscriptionServer) use them at every existing catch site, plus two sites that previously let the exception escape uncaught instead of reaching the peer (CultNetDatabaseServer's v1 subscribe snapshot branch, CultNetDatabaseSubscriptionServer's Subscribe around CreateProjectedSnapshot). CultNetSelectionCursorException now carries AsOf/Current for cursor_stale. R-P: CultNetDatabaseSubscriptionServer.CreateMatchedRecord (the live fast path) authorized against descriptor.SchemaId; the snapshot path (CreateProjectedSnapshot) already authorized against the wire id. Both now call the same CultNetDocumentRegistry.WireSchemaId (made internal, was private) so one id reaches the authorizer on the snapshot, live and reconcile paths alike. R-T(a): CultNetSelectionEvaluator.MatchesSchemaKeysFields no longer reads a Removed change's (always-null) document against Fields - it matches on keys/schemas alone and treats a field predicate as unknown instead of throwing TryGetIndexValue(null). R-T(b): EnsureWithinDeclaredTarget no longer skips the out-of-target check when TargetType is null; it checks against every registered leaf (unconstrained is a real, checked target set, not a bypass). CultCache's ResolveReferenceTarget also now infers the target from a many reference's CultRecordRef element when no explicit [CultReference(typeof(...))] is declared, which was the one remaining source of a null TargetType reaching that check on a real, testable path. Co-Authored-By: Claude Sonnet 5 --- contracts/cultnet/cultnet.error.schema.json | 18 +++++- src/GameCult.Caching/CultCache.cs | 24 +++++++ .../CultNetDatabaseServer.cs | 38 ++++++++--- .../CultNetDatabaseSubscriptionServer.cs | 24 ++++++- .../CultNetDocumentRegistry.cs | 10 ++- .../CultNetSchemaMessages.cs | 64 +++++++++++++++++++ .../CultNetSelectionEvaluator.cs | 31 +++++++-- 7 files changed, 191 insertions(+), 18 deletions(-) diff --git a/contracts/cultnet/cultnet.error.schema.json b/contracts/cultnet/cultnet.error.schema.json index 08dc1e45..e4277f3c 100644 --- a/contracts/cultnet/cultnet.error.schema.json +++ b/contracts/cultnet/cultnet.error.schema.json @@ -10,6 +10,22 @@ "additionalProperties": false, "properties": { "schemaVersion": { "const": "cultnet.error.v0" }, - "error": { "type": "string", "minLength": 1 } + "error": { "type": "string", "minLength": 1 }, + "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"] + }, + "details": { + "description": "Code-specific structured detail (R-N). Every field is populated only for the code that defines it.", + "type": "object", + "additionalProperties": false, + "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 } + } + } } } diff --git a/src/GameCult.Caching/CultCache.cs b/src/GameCult.Caching/CultCache.cs index ddde6349..b75d9a45 100644 --- a/src/GameCult.Caching/CultCache.cs +++ b/src/GameCult.Caching/CultCache.cs @@ -1538,6 +1538,30 @@ namespace GameCult.Caching return memberType.GetGenericArguments()[0]; } + // R-T(b): a many reference's implicit target is its CultRecordRef element's T - the + // same narrowing a scalar CultRecordRef gets above - not left null just because the + // member's own declared type is List<> or IDictionary<,>, not CultRecordRef<> itself. + // Before this, an implicitly-typed many reference reached CultNetSelectionEvaluator's + // EnsureWithinDeclaredTarget with a null TargetType and its out-of-target check was + // silently skipped for every edge through it (R-T(b)/S18). + if (IsEnumerableOfRecordRef(memberType)) + { + foreach (var candidate in memberType.GetInterfaces().Append(memberType)) + { + if (candidate.IsGenericType && + candidate.GetGenericTypeDefinition() == typeof(IEnumerable<>) && + IsCultRecordRefType(candidate.GetGenericArguments()[0])) + { + return candidate.GetGenericArguments()[0].GetGenericArguments()[0]; + } + } + } + + if (TryGetDictionaryRefTypes(memberType, out var keyProperty, out _)) + { + return keyProperty.PropertyType.GetGenericArguments()[0]; + } + return null; } } diff --git a/src/GameCult.Networking/CultNetDatabaseServer.cs b/src/GameCult.Networking/CultNetDatabaseServer.cs index c2952ef8..3c6f89db 100644 --- a/src/GameCult.Networking/CultNetDatabaseServer.cs +++ b/src/GameCult.Networking/CultNetDatabaseServer.cs @@ -245,15 +245,15 @@ namespace GameCult.Networking } catch (CultNetSelectionInvalidException ex) { - peer.SendCultNet(new CultNetErrorMessage { Error = $"selection_invalid: {ex.Message}" }); + peer.SendCultNet(CultNetErrorMessage.ForSelectionInvalid(ex)); } catch (CultNetSelectionCursorException ex) { - peer.SendCultNet(new CultNetErrorMessage { Error = $"{ex.Code}: {ex.Message}" }); + peer.SendCultNet(CultNetErrorMessage.ForCursor(ex)); } catch (CultNetSelectionReferenceOutsideTargetException ex) { - peer.SendCultNet(new CultNetErrorMessage { Error = $"reference_outside_target: {ex.Message}" }); + peer.SendCultNet(CultNetErrorMessage.ForReferenceOutsideTarget(ex)); } return Task.CompletedTask; @@ -379,7 +379,12 @@ namespace GameCult.Networking // refused here rather than reaching Matches' own InvalidOperationException on the first change. if (message.Selection.HasHop) { - peer.SendCultNet(new CultNetErrorMessage { Error = "selection_invalid: selection.cites/selection.cited need the subscription server's reconcile loop (D6); this server only fast-matches a single row." }); + peer.SendCultNet(new CultNetErrorMessage + { + Error = "selection_invalid: selection.cites/selection.cited need the subscription server's reconcile loop (D6); this server only fast-matches a single row.", + Code = "selection_invalid", + Details = new CultNetErrorDetails { Field = "cites" } + }); return Task.CompletedTask; } @@ -389,7 +394,7 @@ namespace GameCult.Networking } catch (CultNetSelectionInvalidException ex) { - peer.SendCultNet(new CultNetErrorMessage { Error = $"selection_invalid: {ex.Message}" }); + peer.SendCultNet(CultNetErrorMessage.ForSelectionInvalid(ex)); return Task.CompletedTask; } @@ -405,11 +410,26 @@ namespace GameCult.Networking if (message.IncludeSnapshot) { - peer.SendCultNet(CreateSelectionResponse(new CultNetSnapshotRequestV1Message + try { - MessageId = message.MessageId, - Selection = message.Selection - })); + peer.SendCultNet(CreateSelectionResponse(new CultNetSnapshotRequestV1Message + { + MessageId = message.MessageId, + Selection = message.Selection + })); + } + catch (CultNetSelectionInvalidException ex) + { + peer.SendCultNet(CultNetErrorMessage.ForSelectionInvalid(ex)); + } + catch (CultNetSelectionCursorException ex) + { + peer.SendCultNet(CultNetErrorMessage.ForCursor(ex)); + } + catch (CultNetSelectionReferenceOutsideTargetException ex) + { + peer.SendCultNet(CultNetErrorMessage.ForReferenceOutsideTarget(ex)); + } } else { diff --git a/src/GameCult.Networking/CultNetDatabaseSubscriptionServer.cs b/src/GameCult.Networking/CultNetDatabaseSubscriptionServer.cs index 4bafe8f3..2ccb031c 100644 --- a/src/GameCult.Networking/CultNetDatabaseSubscriptionServer.cs +++ b/src/GameCult.Networking/CultNetDatabaseSubscriptionServer.cs @@ -155,7 +155,7 @@ namespace GameCult.Networking } catch (CultNetSelectionInvalidException ex) { - peer.SendCultNet(new CultNetErrorMessage { Error = $"selection_invalid: {ex.Message}" }); + peer.SendCultNet(CultNetErrorMessage.ForSelectionInvalid(ex)); return Task.CompletedTask; } @@ -190,6 +190,20 @@ namespace GameCult.Networking PublishDemand(request, key, active: true); projection.DemandActive = true; } + catch (CultNetSelectionInvalidException ex) + { + // R-N: a hop-bearing selection re-validates inside CreateProjectedSnapshot's + // EvaluateAll, so a selection that passed the door check above can still refuse here. + Withdraw(key, sendRemovals: false, forgetRequest: true); + peer.SendCultNet(CultNetErrorMessage.ForSelectionInvalid(ex)); + return Task.CompletedTask; + } + catch (CultNetSelectionReferenceOutsideTargetException ex) + { + Withdraw(key, sendRemovals: false, forgetRequest: true); + peer.SendCultNet(CultNetErrorMessage.ForReferenceOutsideTarget(ex)); + return Task.CompletedTask; + } catch { Withdraw(key, sendRemovals: false, forgetRequest: true); @@ -515,7 +529,13 @@ namespace GameCult.Networking if (!CultNetSelectionEvaluator.Matches(descriptor, key, document, effectiveSelection)) return null; var legacyRequest = request.ToLegacyShape(request.SubscriptionId); - if (_authorizeRecord?.Invoke(legacyRequest, peer, key.Value, descriptor.SchemaId) == false) + // R-P: one id reaches the authorizer, and it is the wire id, on every path. The snapshot + // path (CreateProjectedSnapshot, below) authorizes against the raw record's own SchemaId, + // which is the wire id ToRawRecord/binding emit; this live fast path used to pass + // descriptor.SchemaId instead - the CLR type's own registered id, not the id a binding + // overrides to - so a row delivered by the snapshot could be refused on its first live + // change, or leak through a denylist keyed on the wire id. + if (_authorizeRecord?.Invoke(legacyRequest, peer, key.Value, _database.Documents.WireSchemaId(descriptor)) == false) return null; if (kind == CultNetDatabaseChangeKind.Removed || document == null) diff --git a/src/GameCult.Networking/CultNetDocumentRegistry.cs b/src/GameCult.Networking/CultNetDocumentRegistry.cs index 6d2c778a..1cf0a13c 100644 --- a/src/GameCult.Networking/CultNetDocumentRegistry.cs +++ b/src/GameCult.Networking/CultNetDocumentRegistry.cs @@ -549,8 +549,14 @@ namespace GameCult.Networking return wire; } - /// The wire schema id this registry answers with for a descriptor - its binding's override when one is registered, else the descriptor's own id (R-E: the same id emits). - private string WireSchemaId(CultDocumentDescriptor descriptor) => + /// + /// The wire schema id this registry answers with for a descriptor - its binding's override when + /// one is registered, else the descriptor's own id (R-E: the same id + /// emits). + /// Internal rather than private (R-P): one id reaches the authorizer, and it is this one, on + /// every path - CultNetDatabaseSubscriptionServer's live path needs it too. + /// + internal string WireSchemaId(CultDocumentDescriptor descriptor) => GetByDocumentType(descriptor.DocumentType)?.SchemaId ?? descriptor.SchemaId; /// diff --git a/src/GameCult.Networking/CultNetSchemaMessages.cs b/src/GameCult.Networking/CultNetSchemaMessages.cs index 4498f863..bc2b2379 100644 --- a/src/GameCult.Networking/CultNetSchemaMessages.cs +++ b/src/GameCult.Networking/CultNetSchemaMessages.cs @@ -509,6 +509,70 @@ namespace GameCult.Networking /// Gets or sets optional shard routing details for authority failures. /// [Key("routingHint")] public CultNetShardRoutingHint? RoutingHint { get; set; } + /// + /// Gets or sets the machine-readable refusal code (R-N), e.g. selection_invalid, + /// cursor_stale, cursor_invalid, reference_outside_target. Additive to + /// cultnet.error.v0; a v0 peer that does not know this field ignores it. + /// + [Key("code")] public string? Code { get; set; } + /// + /// Gets or sets the code-specific structured detail (R-N). Additive to cultnet.error.v0. + /// + [Key("details")] public CultNetErrorDetails? Details { get; set; } + + /// + /// Builds the wire error for a door refusal (R-N): code selection_invalid, details + /// { field, value }. + /// + public static CultNetErrorMessage ForSelectionInvalid(CultNetSelectionInvalidException ex) => + new CultNetErrorMessage + { + Error = $"selection_invalid: {ex.Message}", + Code = "selection_invalid", + Details = new CultNetErrorDetails { Field = ex.Field, Value = ex.Value } + }; + + /// + /// Builds the wire error for a cursor refusal (R-N): code cursor_stale with details + /// { asOf, current }, or code cursor_invalid with no details. + /// + public static CultNetErrorMessage ForCursor(CultNetSelectionCursorException ex) => + new CultNetErrorMessage + { + Error = $"{ex.Code}: {ex.Message}", + Code = ex.Code, + Details = ex.Code == "cursor_stale" + ? new CultNetErrorDetails { AsOf = ex.AsOf, Current = ex.Current } + : null + }; + + /// + /// Builds the wire error for an out-of-target reference refusal (R-N): code + /// reference_outside_target, no details. + /// + public static CultNetErrorMessage ForReferenceOutsideTarget(CultNetSelectionReferenceOutsideTargetException ex) => + new CultNetErrorMessage + { + Error = $"reference_outside_target: {ex.Message}", + Code = "reference_outside_target" + }; + } + + /// + /// Code-specific structured detail carried on (R-N). Fields are + /// populated only for the code that defines them; every field is otherwise omitted (nil on the wire). + /// + [MessagePackObject] + public class CultNetErrorDetails + { + /// The selection field that failed (selection_invalid). + [Key("field")] public string? Field { get; set; } + /// The offending value, when there is one string worth naming (selection_invalid). + [Key("value")] public string? Value { get; set; } + /// The cursor's own minted asOf (cursor_stale). + [Key("asOf")] public ulong? AsOf { get; set; } + /// The answering server's current asOf (cursor_stale). + [Key("current")] public ulong? Current { get; set; } } /// diff --git a/src/GameCult.Networking/CultNetSelectionEvaluator.cs b/src/GameCult.Networking/CultNetSelectionEvaluator.cs index cbaaf3c0..aaeeacec 100644 --- a/src/GameCult.Networking/CultNetSelectionEvaluator.cs +++ b/src/GameCult.Networking/CultNetSelectionEvaluator.cs @@ -91,6 +91,12 @@ namespace GameCult.Networking return false; if (selection.Fields == null) return true; + // R-T(a): a removed change is evaluated without reading a document (there is none to read - + // the row is gone). It matches on the selection's keys and schemas alone; a field predicate + // cannot be tested against a value that no longer exists, so it counts as unknown rather + // than excluding the row (or throwing TryGetIndexValue(null)). + if (document == null) + return true; foreach (var field in selection.Fields) { @@ -239,7 +245,11 @@ namespace GameCult.Networking if (cursor.Digest != CultNetSelectionCursor.ComputeDigest(selection)) throw new CultNetSelectionCursorException("cursor_invalid", "The cursor's selection digest does not match this selection."); if (cursor.AsOf != asOf) - throw new CultNetSelectionCursorException("cursor_stale", $"The cursor was minted at asOf {cursor.AsOf}; this server answers as of {asOf}."); + throw new CultNetSelectionCursorException( + "cursor_stale", + $"The cursor was minted at asOf {cursor.AsOf}; this server answers as of {asOf}.", + asOf: cursor.AsOf, + current: asOf); startIndex = FindCursorPosition(ordered, cursor, selection.Descending); } @@ -305,10 +315,14 @@ namespace GameCult.Networking // D9: a reference's target set is every registered leaf assignable to its declared target type. // A stored edge naming a row whose schema is outside that set refuses the selection (S18). + // R-T(b): this runs for every edge, including one whose member carries no declared target type + // (CultDocumentRegistry.PersistedMember.TargetType is null only for a shape the cache could not + // infer a target from - it no longer special-cases that away here without checking anything; + // typeof(object) is every registered leaf, which is the correct target set for a genuinely + // unconstrained reference and keeps this the one path, not a skip plus a duplicate path. private static void EnsureWithinDeclaredTarget(CultDocumentRegistry registry, Type? targetType, Row from, string role, Row to) { - if (targetType == null) return; - var leaves = registry.ResolveTargetLeaves(targetType); + var leaves = registry.ResolveTargetLeaves(targetType ?? typeof(object)); if (leaves.Any(leaf => leaf.SchemaId == to.Descriptor.SchemaId)) return; throw new CultNetSelectionReferenceOutsideTargetException(from.Descriptor.SchemaId, from.Key.Value, role, to.Descriptor.SchemaId, to.Key.Value); } @@ -407,12 +421,21 @@ namespace GameCult.Networking /// The typed refusal for a cursor the server cannot answer (cursor_stale/cursor_invalid). public sealed class CultNetSelectionCursorException : Exception { - public CultNetSelectionCursorException(string code, string message) : base(message) + public CultNetSelectionCursorException(string code, string message, ulong? asOf = null, ulong? current = null) + : base(message) { Code = code; + AsOf = asOf; + Current = current; } public string Code { get; } + + /// The cursor's own minted asOf. Carried on the wire only for cursor_stale (R-N). + public ulong? AsOf { get; } + + /// The answering server's current asOf. Carried on the wire only for cursor_stale (R-N). + public ulong? Current { get; } } ///