diff --git a/src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-26-a-declaration-can-no-longer-write-itself-as-an-instance.md b/src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-26-a-declaration-can-no-longer-write-itself-as-an-instance.md new file mode 100644 index 0000000000..520e55f865 --- /dev/null +++ b/src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-26-a-declaration-can-no-longer-write-itself-as-an-instance.md @@ -0,0 +1,26 @@ +--- +Name: A node type declaration can no longer write itself as an instance +Category: Fix +Description: Creating or updating a node type declaration that also claims to be one of its own instances is now refused at write time, closing the class of bug behind the user-directory type-confusion incidents for good. +Icon: PersonQuestionMark +Order: -20260826 +--- + +# A node type declaration can no longer write itself as an instance + +A recent fix retyped the three built-in declarations that had been filed under the type they +describe — `User`, `Virtual User` and `Partition` — so they stopped showing up in their own +`nodeType:` queries. That cleaned up the three known cases, but nothing stopped the same mistake +from being written again later: a repair, a package install, or a hand-authored edit could still +create a new declaration that files itself under its own type, and the portal's user directory would +go right back to handing out a description-of-a-person where it expected an actual person. + +Creating or updating a node now checks that rule directly: a node whose content *describes* a type +can no longer also file itself under **that same type**. The write is refused with a message naming +the node and the type it claims, rather than surfacing later as a silent lookup failure. This closes +the underlying class of bug rather than the three instances of it that were already fixed. + +The rule is deliberately narrow. A node that describes a type while being filed under some +*unrelated* type is still allowed — a package root that is a space, and whose content happens to +also describe a type, is a real and shipping shape. Refusing that would have made those packages +impossible to install, which is a worse outcome than the mismatch it would have prevented. diff --git a/src/MeshWeaver.Graph/Configuration/GraphConfigurationExtensions.cs b/src/MeshWeaver.Graph/Configuration/GraphConfigurationExtensions.cs index 6153005102..bd25cda6b4 100644 --- a/src/MeshWeaver.Graph/Configuration/GraphConfigurationExtensions.cs +++ b/src/MeshWeaver.Graph/Configuration/GraphConfigurationExtensions.cs @@ -228,6 +228,16 @@ public TBuilder AddGraph() // IMessageHub, so the registry checked is the validating hub's own chain. services.AddScoped(); + // Write-boundary guard for the OTHER collision class in the same family + // (#2160/#2161/#2162, #2245, #2358): a NodeType declaration (Content IS a + // NodeTypeDefinition) must never also claim, via its own MeshNode.NodeType, to + // be an INSTANCE of a type. #2245 retyped the three known offenders and added a + // static ratchet test, but nothing stopped a FUTURE create/update (a repair + // path, a plugin install) from reintroducing the same collision at runtime, + // where a static-registration-only ratchet cannot see it. Stateless, like + // CodeNodeSegmentNameValidator below. + services.AddScoped(); + // Keeps the batch bake's "the union is every Code node" claim true BY // CONSTRUCTION (issue #1235): a Code node named after a code-table routing // segment (Source/Test) lands in the code table but has a namespace that diff --git a/src/MeshWeaver.Graph/Security/NodeTypeDeclarationSelfTypingValidator.cs b/src/MeshWeaver.Graph/Security/NodeTypeDeclarationSelfTypingValidator.cs new file mode 100644 index 0000000000..a37141b04c --- /dev/null +++ b/src/MeshWeaver.Graph/Security/NodeTypeDeclarationSelfTypingValidator.cs @@ -0,0 +1,136 @@ +using System.Reactive.Linq; +using System.Text.Json; +using MeshWeaver.Mesh; +using MeshWeaver.Mesh.Security; +using MeshWeaver.Mesh.Services; + +namespace MeshWeaver.Graph.Security; + +/// +/// Fail-closed guard for Create/Update: refuses a write that DECLARES a NodeType — its +/// IS a — while also +/// naming ITSELF via , i.e. enrolling the declaration in its own +/// instance query. +/// +/// The defect this closes at the WRITER (#2160/#2161/#2162, #2245, #2358). A NodeType +/// declaration is the MeshNode that DEFINES a type. Three built-in declarations (User, +/// VUser, Partition) shipped with set to their OWN +/// name — i.e. each claimed to also BE an instance of the type it defines. That makes a declaration +/// indistinguishable from a real instance to every nodeType:<Type> query in the mesh: +/// the portal's user directory (UserIdentityCache) returned the User declaration +/// alongside real accounts and logged As<User> for User: value is NodeTypeDefinition on +/// every index snapshot — 355k+ occurrences in production. #2245 retyped the three known offenders +/// (NodeType = MeshNode.NodeTypePath, exactly as Space/Release/Build +/// already did) and added a STATIC ratchet test over every built-in declaration +/// (NodeTypeDeclarationSelfTypingTest) — but nothing stopped a FUTURE write (a repair path, a +/// plugin-installed NodeType, a hand-authored patch) from reintroducing the exact same collision at +/// RUNTIME, which a static-registration-only ratchet cannot see. #2358 reported the identical +/// signature from a different host category, which is exactly the shape of recurrence a +/// retroactive, instance-by-instance fix cannot close. This validator is the "ONE change that fixes +/// the class of bug rather than an instance of it" #2358 itself asks for: refuse the collision AT +/// THE WRITE BOUNDARY, for every declaration present or future, not just the three named so far. +/// +/// 🚨 SELF-typing only — a declaration naming an UNRELATED type stays legal, and that is +/// deliberate. The guard first refused any NodeType other than NodeType, and that +/// broke a shape the plugins repo actually ships: a package ROOT that is a Space and whose +/// content also happens to be a NodeTypeDefinition (the UWDeepfield shape, pinned by +/// NodeRepoInstanceOrderingTest). Refusing it made the whole package un-installable — a +/// strictly worse outcome than the degradation it was guarding against, and one CI caught before +/// merge. The harm this guard exists to stop is a declaration polluting the instance query for the +/// type IT DECLARES (User declaration answering nodeType:User beside real accounts); +/// a Space root answering nodeType:Space is simply a Space, correctly returned. +/// +/// The residual is real but SMALLER and is NOT closed here: a nodeType:Space consumer +/// that calls ContentAs<Space>() on such a root gets a degraded null, because the +/// content is a NodeTypeDefinition. Closing that means changing the shipped package first +/// (a cross-repo change in MeshWeaver.Plugins), not refusing the write here — refusing it at the +/// boundary while the content still ships is how a guard breaks production rather than protecting +/// it. +/// +/// Bad-data TOLERANCE is unchanged: this guard never rejects a READ. An existing row that +/// already carries the collision keeps loading (degraded, tolerated by +/// ) until it is next written; only a NEW create/update that +/// would (re)introduce the collision is refused. +/// +public sealed class NodeTypeDeclarationSelfTypingValidator : INodeValidator +{ + /// Create + Update — the two surfaces that persist a node's own NodeType field. + public IReadOnlyCollection SupportedOperations => + [NodeOperation.Create, NodeOperation.Update]; + + /// + /// Rejects a create/update whose content declares a NodeType while its own + /// names an instance type — unset or + /// are both legal for a declaration. + /// + /// The validation context describing the node and operation. + /// An observable emitting the validation result. + public IObservable Validate(NodeValidationContext context) + { + var node = context.Node; + + // Nothing to judge unless the NodeType field claims to be an INSTANCE of something. + if (string.IsNullOrEmpty(node.NodeType) + || string.Equals(node.NodeType, MeshNode.NodeTypePath, StringComparison.Ordinal)) + return Observable.Return(NodeValidationResult.Valid()); + + if (!IsNodeTypeDeclarationContent(node.Content)) + return Observable.Return(NodeValidationResult.Valid()); + + // SELF-typing only: the NodeType field names THIS declaration. An instance references a + // type by the declaration's path (`nodeType:"Pack/Widget"`) or, for a built-in at the root, + // by its id (`nodeType:"User"`) — so those two are the ways a declaration can enrol itself + // in its own instance query. Naming an UNRELATED type is a different shape and is legal + // (see the class doc). + if (!string.Equals(node.NodeType, node.Path, StringComparison.OrdinalIgnoreCase) + && !string.Equals(node.NodeType, node.Id, StringComparison.OrdinalIgnoreCase)) + return Observable.Return(NodeValidationResult.Valid()); + + return Observable.Return(NodeValidationResult.Invalid( + $"'{node.Path}' carries NodeTypeDefinition content (it DECLARES a NodeType) and its own " + + $"NodeType field is '{node.NodeType}' — i.e. the declaration claims to be an INSTANCE OF " + + "ITSELF. That makes it indistinguishable from a real instance to every " + + $"'nodeType:{node.NodeType}' query in the mesh. Set NodeType to " + + $"'{MeshNode.NodeTypePath}', or leave it unset.", + NodeRejectionReason.ValidationFailed)); + } + + /// + /// True when is a — + /// already typed (in-process content), or degraded to the raw JSON a cross-hub write can arrive + /// as (the same discriminator shape ContentDiscriminatorValidator reads for its own + /// $type probe). + /// + private static bool IsNodeTypeDeclarationContent(object? content) => content switch + { + Configuration.NodeTypeDefinition => true, + JsonElement je when je.ValueKind == JsonValueKind.Object + && je.TryGetProperty("$type", out var discriminator) + && discriminator.ValueKind == JsonValueKind.String + && IsNodeTypeDefinitionDiscriminator(discriminator.GetString()) => true, + _ => false, + }; + + /// + /// True when names + /// — as a bare short name, a namespace-qualified name, OR the full CLR + /// AssemblyQualifiedName shape ("Namespace.Type, AssemblyName, Version=…"). + /// Hand-authored JSON (including MCP writes) is not guaranteed to use the framework's own + /// "Namespace.Type" convention — an assembly-qualified $type that the framework's own + /// writers never produce would otherwise fail the EndsWith check (it ends with + /// ", AssemblyName", not ".NodeTypeDefinition") and let the guard fail OPEN. + /// + private static bool IsNodeTypeDefinitionDiscriminator(string? discriminator) + { + if (string.IsNullOrEmpty(discriminator)) + return false; + + // Strip a trailing assembly qualifier ("…, AssemblyName, Version=…, Culture=…, …") before + // comparing the type-name portion — comma-separated, never present in the bare/namespaced + // shapes this compares against. + var typeName = discriminator.Split(',')[0].Trim(); + + return string.Equals(typeName, nameof(Configuration.NodeTypeDefinition), StringComparison.Ordinal) + || typeName.EndsWith("." + nameof(Configuration.NodeTypeDefinition), StringComparison.Ordinal); + } +} diff --git a/test/MeshWeaver.Graph.Test/NodeTypeDeclarationSelfTypingValidatorTest.cs b/test/MeshWeaver.Graph.Test/NodeTypeDeclarationSelfTypingValidatorTest.cs new file mode 100644 index 0000000000..49057d1e47 --- /dev/null +++ b/test/MeshWeaver.Graph.Test/NodeTypeDeclarationSelfTypingValidatorTest.cs @@ -0,0 +1,233 @@ +using System.Linq; +using System.Text.Json; +using System.Threading.Tasks; +using MeshWeaver.Graph.Configuration; +using MeshWeaver.Graph.Security; +using MeshWeaver.Hosting.Monolith.TestBase; +using MeshWeaver.Mesh; +using MeshWeaver.Mesh.Security; +using MeshWeaver.Mesh.Services; +using Microsoft.Extensions.DependencyInjection; +using Xunit; + +namespace MeshWeaver.Graph.Test; + +/// +/// Pins — the write-boundary half of the fix +/// for #2160/#2161/#2162/#2245/#2358: a NodeType declaration (content IS a +/// ) must never also claim, via its own +/// , to be an INSTANCE of a type. +/// +/// #2245 retyped the three known offenders (User, VUser, Partition) and +/// added a STATIC ratchet test (NodeTypeDeclarationSelfTypingTest) over every +/// statically-registered declaration. That ratchet cannot see a declaration created or patched at +/// RUNTIME — a repair path, a plugin-installed NodeType, a hand-authored MCP write. This validator +/// closes that gap at the one place every such write passes through: Create/Update validation. +/// +public class NodeTypeDeclarationSelfTypingValidatorTest(ITestOutputHelper output) : MonolithMeshTestBase(output) +{ + private NodeTypeDeclarationSelfTypingValidator Guard => + Mesh.ServiceProvider.GetServices() + .OfType() + .Single(); + + private static MeshNode DeclarationNode(string nodeType, object? content) => new("Widget") + { + Name = "Widget", + NodeType = nodeType, + Content = content, + }; + + [Fact(Timeout = 30000)] + public async Task ADeclaration_ClaimingToBeAnInstanceOfTheTypeItDeclares_IsRejected() + { + // The exact production shape: NodeType == the name the declaration itself introduces. + var result = await Guard + .Validate(new NodeValidationContext + { + Operation = NodeOperation.Create, + Node = DeclarationNode("Widget", new NodeTypeDefinition()), + }) + .Should().Emit(); + + result.IsValid.Should().BeFalse( + "content is a NodeTypeDefinition — this node DECLARES 'Widget' — so it must not also " + + "claim (via MeshNode.NodeType) to BE an instance of 'Widget', or anything else"); + result.ErrorMessage.Should().Contain("Widget"); + } + + /// + /// 🚨 A declaration naming an UNRELATED type must be ACCEPTED. This test asserted the opposite + /// until CI proved that wrong: the broad rule refused a shape MeshWeaver.Plugins actually ships + /// — a package ROOT that is a Space whose content also happens to be a + /// NodeTypeDefinition (the UWDeepfield shape) — which made the whole package + /// un-installable. NodeRepoInstanceOrderingTest pins that install and went red. + /// + /// The harm being guarded is a declaration polluting the instance query for the type IT + /// DECLARES: the User declaration answering nodeType:User beside real accounts + /// (355k+ production occurrences). A Space root answering nodeType:Space is + /// simply a Space, correctly returned — no collision to prevent. + /// + /// The residual is acknowledged in the validator's own docs and deliberately NOT closed + /// here: a ContentAs<Space>() on such a root degrades to null. Fixing that means + /// changing the shipped package first, not refusing the write while the content still ships. + /// + [Fact(Timeout = 30000)] + public async Task ADeclaration_NamingAnUnrelatedType_IsAccepted() + { + var result = await Guard + .Validate(new NodeValidationContext + { + Operation = NodeOperation.Create, + // The UWDeepfield shape: a Space root whose content is a NodeTypeDefinition. + Node = DeclarationNode("Space", new NodeTypeDefinition()), + }) + .Should().Emit(); + + result.IsValid.Should().BeTrue( + "a declaration naming an UNRELATED type does not enrol itself in its own instance " + + "query, and refusing it makes the shipped UWDeepfield package un-installable, " + + $"got: {result.ErrorMessage}"); + } + + /// + /// The self-typing check must match on the declaration's PATH too, not only its id — an + /// instance references an in-package type by path (nodeType:"Pack/Widget"), so a + /// declaration at that path naming it is the same self-enrolment as the root-level case. + /// + [Fact(Timeout = 30000)] + public async Task ADeclaration_SelfTypedByItsFullPath_IsRejected() + { + var result = await Guard + .Validate(new NodeValidationContext + { + Operation = NodeOperation.Create, + Node = new MeshNode("Widget", "Pack") + { + Name = "Widget", + NodeType = "Pack/Widget", + Content = new NodeTypeDefinition(), + }, + }) + .Should().Emit(); + + result.IsValid.Should().BeFalse( + "instances reference this type as 'Pack/Widget', so the declaration naming that path " + + "puts itself in its own instance query — the id-only check would miss it"); + } + + [Fact(Timeout = 30000)] + public async Task ADeclaration_TypedAsNodeTypePath_IsAccepted() + { + var result = await Guard + .Validate(new NodeValidationContext + { + Operation = NodeOperation.Create, + Node = DeclarationNode(MeshNode.NodeTypePath, new NodeTypeDefinition()), + }) + .Should().Emit(); + + result.IsValid.Should().BeTrue( + $"NodeType == '{MeshNode.NodeTypePath}' is exactly how a declaration should say what " + + $"it is, got: {result.ErrorMessage}"); + } + + [Fact(Timeout = 30000)] + public async Task ADeclaration_WithNoNodeTypeSet_IsAccepted() + { + var result = await Guard + .Validate(new NodeValidationContext + { + Operation = NodeOperation.Create, + Node = DeclarationNode(null!, new NodeTypeDefinition()), + }) + .Should().Emit(); + + result.IsValid.Should().BeTrue("an unset NodeType is also legal for a declaration"); + } + + /// + /// The degraded shape a cross-hub write can arrive in — a raw whose + /// $type is whatever the WRITING hub's registry named the CLR type. Same probe shape as + /// ContentDiscriminatorValidator's own $type read. + /// + [Fact(Timeout = 30000)] + public async Task ACrossHubWrite_CarryingTheDegradedNodeTypeDefinitionShape_IsAlsoRejected() + { + var content = JsonSerializer.Deserialize( + """{"$type":"NodeTypeDefinition","description":"a widget type"}"""); + + var result = await Guard + .Validate(new NodeValidationContext + { + Operation = NodeOperation.Create, + Node = DeclarationNode("Widget", content), + }) + .Should().Emit(); + + result.IsValid.Should().BeFalse( + "the degraded JsonElement shape must be recognised as declaration content too — a " + + "cross-hub writer must not be able to smuggle the collision past the guard"); + } + + /// + /// Hand-authored JSON (an MCP write, in particular) is not guaranteed to use the framework's + /// own "Namespace.Type" $type convention — it may carry the full CLR AssemblyQualifiedName + /// shape instead ("Namespace.Type, AssemblyName, Version=…"). Flagged in review (#2378): a + /// naive EndsWith(".NodeTypeDefinition") check fails on this shape, since it ends with + /// ", AssemblyName" — which would let the guard fail OPEN for exactly this kind of write. + /// + [Fact(Timeout = 30000)] + public async Task ACrossHubWrite_CarryingTheAssemblyQualifiedDiscriminatorShape_IsAlsoRejected() + { + var content = JsonSerializer.Deserialize( + """ + {"$type":"MeshWeaver.Graph.Configuration.NodeTypeDefinition, MeshWeaver.Graph, Version=1.0.0.0", + "description":"a widget type"} + """); + + var result = await Guard + .Validate(new NodeValidationContext + { + Operation = NodeOperation.Create, + Node = DeclarationNode("Widget", content), + }) + .Should().Emit(); + + result.IsValid.Should().BeFalse( + "an assembly-qualified $type discriminator must be recognised as declaration content " + + "too, or a hand-authored/MCP write in this shape slips the guard entirely"); + } + + /// The guard must not widen into rejecting ordinary instances of ordinary types. + [Fact(Timeout = 30000)] + public async Task AnOrdinaryInstanceWrite_IsUnaffected() + { + var result = await Guard + .Validate(new NodeValidationContext + { + Operation = NodeOperation.Create, + // Content is NOT a NodeTypeDefinition — an ordinary instance of "Widget". + Node = DeclarationNode("Widget", new { name = "a widget" }), + }) + .Should().Emit(); + + result.IsValid.Should().BeTrue( + $"content is not a NodeTypeDefinition, so this is an ordinary instance write and must " + + $"pass, got: {result.ErrorMessage}"); + } + + /// + /// End-to-end: the validator is actually WIRED into the mesh's Create/Update pipeline (not just + /// unit-callable), so a genuine self-typed declaration write is refused by the running mesh — + /// not merely by calling the class directly. + /// + [Fact(Timeout = 30000)] + public async Task TheGuard_IsWiredIntoTheLiveCreatePipeline() + { + var validators = Mesh.ServiceProvider.GetServices().ToList(); + validators.OfType().Should().ContainSingle( + "AddGraph() must register the guard, or a runtime self-typed declaration write " + + "reaches the mesh unchecked"); + } +}