fix(mtmt): restore .tm7 openability and close the ROOT differential

docs/analysis-rules.md

@@ -212,6 +212,23 @@ The import preserves source expressions and metadata in provenance. A non-strict
 exactly representable threats and reports skipped threats; `--strict` writes nothing if any threat is
 skipped. Generated packs remain subject to the source template's license and attribution terms.
 
+#### `ROOT` rules run once per diagram
+
+An imported threat whose filter is `source is 'ROOT'` — the six migrated STRIDE types `SU`, `TU`,
+`RU`, `IU`, `DU`, and `EU` in Microsoft's default template — is evaluated **once per diagram**, and
+reports the diagram itself as the finding's target.
+
+This is a deliberate difference from the Microsoft Threat Modeling Tool, which generates strictly once
+per interaction and never fires these rules at all: it builds an element's type chain without the
+virtual `ROOT` type at its head, so `source is 'ROOT'` cannot hold for any element. The types are
+inert there, and their own descriptions record them as migrated from version 3.
+
+Threat Model Forge keeps them live because a per-diagram STRIDE sweep is useful coverage, so expect
+these rules to report findings the tool does not. They are Threat Model Forge behavior rather than a
+parity claim. Nothing changes on export: a rule that cannot fire in the tool contributes no threats to
+an exported `.tm7`. The evidence is the committed capture under
+`test/ThreatModelForge.Analysis.Tests/Fixtures/MtmtDifferential/`.
+
 Because a spec is inspectable data — not an assembly — it is safe to share and review, and it runs
 everywhere the CLI does. Custom rules are **added to** the built-in rules, never a replacement for
 them: `--rules` loads your rules *alongside* the full built-in set and both are evaluated together

src/ThreatModelForge.Analysis/InteractionRule.cs

@@ -116,6 +116,14 @@ public override void Evaluate(RuleEvaluationContext context)
             foreach (DrawingSurfaceModel diagram in context.Model.DrawingSurfaceList)
             {
                 context.AccountDeclarativeOperations(diagram.Lines.Count);
+
+                // A deliberate divergence from the Microsoft Threat Modeling Tool, recorded by
+                // MtmtRootDifferentialTests against a capture taken from the pinned tool build. The tool
+                // cannot fire a ROOT predicate at all: it builds an element's type chain without the
+                // virtual ROOT type at its head, so "source is 'ROOT'" is never true and the six
+                // migrated ROOT threat types are inert. Threat Model Forge keeps them live as a
+                // per-diagram STRIDE sweep - coverage the tool lost - so a ROOT rule fires once per
+                // diagram rather than once per interaction.
                 if (this.evaluatesRoot)
                 {
                     InteractionExpression.EvaluationContext root = InteractionExpression.EvaluationContext.Root(diagram);

src/ThreatModelForge.Analysis/KnowledgeBaseCatalog.cs

@@ -219,41 +219,33 @@ private static void AddThreatCategories(List<ThreatCategory> categories, RuleSet
         }
 
         /// <summary>
-        /// Declares the threat priority vocabulary on the knowledge base.
+        /// Declares the threat metadata the Microsoft Threat Modeling Tool owns, including the threat
+        /// priority vocabulary.
         /// </summary>
         /// <remarks>
         /// <para>
-        /// This is declared unconditionally, because every generated threat carries a priority whether
-        /// or not any rule declared a default for it. The Microsoft Threat Modeling Tool drives its
-        /// priority field from <c>IsPriorityUsed</c> and this value list; omitting them leaves the
-        /// priorities Threat Model Forge wrote unmanaged in the tool, where editing a threat can
-        /// quietly replace a value the list does not offer.
+        /// The priority vocabulary is declared unconditionally, because every generated threat carries a
+        /// priority whether or not any rule declared a default for it. The tool drives its priority field
+        /// from <c>IsPriorityUsed</c> and this value list; omitting them leaves the priorities Threat
+        /// Model Forge wrote unmanaged in the tool, where editing a threat can quietly replace a value
+        /// the list does not offer.
         /// </para>
         /// <para>
         /// The declared values are the whole <see cref="ThreatPriority"/> vocabulary, so any priority
         /// Threat Model Forge can express survives a round trip through the tool. This mirrors the
         /// shape of the official Microsoft templates, which declare their vocabulary the same way.
         /// </para>
+        /// <para>
+        /// The rest of the block is the property set the tool resolves by name while loading a model. It
+        /// must be declared alongside the priority vocabulary: the tool tolerates a knowledge base that
+        /// declares no threat metadata at all, but refuses to open one that declares only part of the
+        /// required set. See <see cref="ThreatMetaDataContract"/>.
+        /// </para>
         /// </remarks>
         /// <param name="knowledgeBase">The knowledge base being built.</param>
         private static void AddThreatMetaData(KnowledgeBaseData knowledgeBase)
         {
-            ThreatMetaData metadata = new ThreatMetaData { IsPriorityUsed = true };
-            ThreatMetaDatum priority = new ThreatMetaDatum
-            {
-                Name = "Priority",
-                Label = "Priority",
-                Description = "Generated threat priority.",
-                Id = "tmforge:priority",
-                AttributeType = 1,
-            };
-            foreach (string value in ThreatPriorities.All)
-            {
-                priority.Values.Add(value);
-            }
-
-            metadata.PropertiesMetaData.Add(priority);
-            knowledgeBase.ThreatMetaData = metadata;
+            knowledgeBase.ThreatMetaData = ThreatMetaDataContract.Create(ThreatPriorities.All);
         }
 
         private static ThreatCategory ThreatCategoryFor(string id, string name, string description)

src/ThreatModelForge.Analysis/ThreatMetaDataContract.cs

@@ -0,0 +1,109 @@
+namespace ThreatModelForge.Analysis
+{
+    using System;
+    using System.Collections.Generic;
+    using ThreatModelForge.Model;
+
+    /// <summary>
+    /// Declares the threat metadata the Microsoft Threat Modeling Tool owns.
+    /// </summary>
+    /// <remarks>
+    /// <para>
+    /// The tool resolves six of these properties by name while loading a model
+    /// (<c>ThreatMetaData.FindIndices</c>) and throws
+    /// <c>"KnowledgeBase is missing a required Threat Property in ThreatMetaData"</c> when any of them
+    /// is absent, refusing to open the file. Omitting the whole metadata block is tolerated, but
+    /// declaring a partial one is not, so a knowledge base that declares any threat metadata at all
+    /// must declare the complete required set.
+    /// </para>
+    /// <para>
+    /// These properties are tool-owned rather than authored: their identity is the name, and their
+    /// values are the pickers the tool offers. A foreign template's own labels, identifiers, and value
+    /// lists are therefore authoritative, and Threat Model Forge defers to them on merge instead of
+    /// treating a difference as a conflict.
+    /// </para>
+    /// </remarks>
+    internal static class ThreatMetaDataContract
+    {
+        /// <summary>The threat title.</summary>
+        public const string TitleName = "Title";
+
+        /// <summary>The author-visible threat category.</summary>
+        public const string CategoryName = "UserThreatCategory";
+
+        /// <summary>The short description shown in the threat list.</summary>
+        public const string ShortDescriptionName = "UserThreatShortDescription";
+
+        /// <summary>The long threat description.</summary>
+        public const string DescriptionName = "UserThreatDescription";
+
+        /// <summary>The triage justification.</summary>
+        public const string StateInformationName = "StateInformation";
+
+        /// <summary>The rendered source, flow, and target of the interaction.</summary>
+        public const string InteractionName = "InteractionString";
+
+        /// <summary>The threat priority.</summary>
+        public const string PriorityName = "Priority";
+
+        private static readonly HashSet<string> ToolOwnedNames = new HashSet<string>(StringComparer.OrdinalIgnoreCase)
+        {
+            TitleName,
+            CategoryName,
+            ShortDescriptionName,
+            DescriptionName,
+            StateInformationName,
+            InteractionName,
+            PriorityName,
+        };
+
+        /// <summary>
+        /// Gets a value indicating whether the named property is owned by the tool rather than authored.
+        /// </summary>
+        /// <param name="name">The property name to test.</param>
+        /// <returns><see langword="true"/> when the tool owns the property.</returns>
+        public static bool IsToolOwned(string? name)
+            => !string.IsNullOrEmpty(name) && ToolOwnedNames.Contains(name!);
+
+        /// <summary>
+        /// Builds the complete threat metadata block, declaring every property the tool requires plus
+        /// the priority vocabulary.
+        /// </summary>
+        /// <param name="priorities">The priority vocabulary to offer.</param>
+        /// <returns>A metadata block the tool can load.</returns>
+        public static ThreatMetaData Create(IEnumerable<string> priorities)
+        {
+            ThreatMetaData metadata = new ThreatMetaData { IsPriorityUsed = true };
+            metadata.PropertiesMetaData.Add(Datum(TitleName, "Title", "tmforge:title", 0, false));
+            metadata.PropertiesMetaData.Add(Datum(CategoryName, "Category", "tmforge:category", 0, false));
+            metadata.PropertiesMetaData.Add(
+                Datum(ShortDescriptionName, "Short Description", "tmforge:short-description", 1, true));
+            metadata.PropertiesMetaData.Add(Datum(DescriptionName, "Description", "tmforge:description", 0, false));
+            metadata.PropertiesMetaData.Add(
+                Datum(StateInformationName, "Justification", "tmforge:state-information", 0, false));
+            metadata.PropertiesMetaData.Add(Datum(InteractionName, "Interaction", "tmforge:interaction", 0, false));
+
+            ThreatMetaDatum priority = Datum(PriorityName, "Priority", "tmforge:priority", 1, false);
+            priority.Description = "Generated threat priority.";
+            foreach (string value in priorities)
+            {
+                priority.Values.Add(value);
+            }
+
+            metadata.PropertiesMetaData.Add(priority);
+            return metadata;
+        }
+
+        private static ThreatMetaDatum Datum(string name, string label, string id, int attributeType, bool hideFromUI)
+        {
+            return new ThreatMetaDatum
+            {
+                Name = name,
+                Label = label,
+                Id = id,
+                AttributeType = attributeType,
+                HideFromUI = hideFromUI,
+            };
+        }
+    }
+}

src/ThreatModelForge.Analysis/Tm7ExportPreparer.cs

@@ -32,6 +32,22 @@ public static class Tm7ExportPreparer
         /// </summary>
         private const int MinimumCoordinate = 10;
 
+        /// <summary>
+        /// The largest drawing coordinates the tool accepts, which it applies per element kind: borders
+        /// are clamped to 1890 x 2090 and connector endpoints and handles to 1990 x 2190. Exceeding
+        /// either is "corrected" on open exactly as an under-run is.
+        /// </summary>
+        private const int MaximumBorderX = 1890;
+
+        /// <summary>The largest border ordinate the tool accepts.</summary>
+        private const int MaximumBorderY = 2090;
+
+        /// <summary>The largest connector abscissa the tool accepts.</summary>
+        private const int MaximumLineX = 1990;
+
+        /// <summary>The largest connector ordinate the tool accepts.</summary>
+        private const int MaximumLineY = 2190;
+
         /// <summary>
         /// Ensures the model carries the default knowledge base and has its schema-backed properties
         /// typed, unless it already carries a foreign knowledge base.
@@ -201,12 +217,14 @@ private static void MergeThreatMetadata(
                     continue;
                 }
 
-                // Two knowledge bases can declare different priority vocabularies without being in
-                // conflict: a vocabulary is a set of offered values, so the union is what lets every
-                // priority either side can express stay selectable. Rejecting the difference would fail
-                // the export outright, and taking one side's list would leave threats carrying a value
-                // the tool no longer offers - the silent downgrade this is here to prevent.
-                if (isGlobalVocabulary && matches.Count == 1 && IsPriorityMetadata(datum))
+                // Two knowledge bases can declare the tool's own threat metadata differently without
+                // being in conflict: these properties are identified by name, and their values are the
+                // pickers the tool offers, so the union is what lets every value either side can express
+                // stay selectable. Rejecting the difference would fail the export outright, and taking
+                // one side's list would leave threats carrying a value the tool no longer offers - the
+                // silent downgrade this is here to prevent. The foreign template's own label and
+                // identifier are authoritative, so only the values are folded in.
+                if (isGlobalVocabulary && matches.Count == 1 && ThreatMetaDataContract.IsToolOwned(datum.Name))
                 {
                     UnionValues(matches[0].Values, datum.Values);
                     continue;
@@ -289,10 +307,10 @@ private static bool ThreatMetadataMatches(ThreatMetaDatum left, ThreatMetaDatum
         }
 
         /// <summary>
-        /// Translates each drawing surface as a whole so its lowest element and connector coordinates
-        /// sit at or beyond <see cref="MinimumCoordinate"/>. Shifting the surface rather than clamping
-        /// each element individually preserves the relative layout and keeps connectors attached to
-        /// their endpoints, which a per-element clamp (as the tool itself performs) would not.
+        /// Translates each drawing surface as a whole so its element and connector coordinates sit
+        /// inside the range the tool accepts. Shifting the surface rather than clamping each element
+        /// individually preserves the relative layout and keeps connectors attached to their endpoints,
+        /// which a per-element clamp (as the tool itself performs) would not.
         /// </summary>
         /// <param name="model">The model to normalize; it is mutated in place.</param>
         private static void NormalizeCoordinates(ThreatModel model)
@@ -301,26 +319,36 @@ private static void NormalizeCoordinates(ThreatModel model)
             {
                 int minX = int.MaxValue;
                 int minY = int.MaxValue;
+                int upperX = int.MaxValue;
+                int upperY = int.MaxValue;
 
                 foreach (DrawingElement element in surface.Borders.Values.OfType<DrawingElement>())
                 {
                     minX = Math.Min(minX, element.Left);
                     minY = Math.Min(minY, element.Top);
+                    upperX = Math.Min(upperX, MaximumBorderX - element.Left);
+                    upperY = Math.Min(upperY, MaximumBorderY - element.Top);
                 }
 
                 foreach (LineElement line in surface.Lines.Values.OfType<LineElement>())
                 {
-                    minX = Math.Min(minX, Math.Min(line.SourceX, Math.Min(line.TargetX, line.HandleX)));
-                    minY = Math.Min(minY, Math.Min(line.SourceY, Math.Min(line.TargetY, line.HandleY)));
+                    int lineMinX = Math.Min(line.SourceX, Math.Min(line.TargetX, line.HandleX));
+                    int lineMinY = Math.Min(line.SourceY, Math.Min(line.TargetY, line.HandleY));
+                    int lineMaxX = Math.Max(line.SourceX, Math.Max(line.TargetX, line.HandleX));
+                    int lineMaxY = Math.Max(line.SourceY, Math.Max(line.TargetY, line.HandleY));
+                    minX = Math.Min(minX, lineMinX);
+                    minY = Math.Min(minY, lineMinY);
+                    upperX = Math.Min(upperX, MaximumLineX - lineMaxX);
+                    upperY = Math.Min(upperY, MaximumLineY - lineMaxY);
                 }
 
                 if (minX == int.MaxValue)
                 {
                     continue;
                 }
 
-                int deltaX = minX < MinimumCoordinate ? MinimumCoordinate - minX : 0;
-                int deltaY = minY < MinimumCoordinate ? MinimumCoordinate - minY : 0;
+                int deltaX = ShiftInto(minX, upperX);
+                int deltaY = ShiftInto(minY, upperY);
                 if (deltaX == 0 && deltaY == 0)
                 {
                     continue;
@@ -345,5 +373,29 @@ private static void NormalizeCoordinates(ThreatModel model)
                 }
             }
         }
+
+        /// <summary>
+        /// Chooses the translation that brings one axis of a surface inside the tool's range, given the
+        /// lowest coordinate on that axis and the largest shift its highest coordinates still allow.
+        /// </summary>
+        /// <remarks>
+        /// A surface drawn wider than the tool's canvas cannot satisfy both bounds by translation. It is
+        /// anchored at the low edge instead, because scaling it to fit would change the geometry that
+        /// trust-boundary containment is derived from, which would alter the analysis rather than the
+        /// drawing.
+        /// </remarks>
+        /// <param name="minimum">The lowest coordinate present on the axis.</param>
+        /// <param name="headroom">The largest shift the highest coordinates on the axis permit.</param>
+        /// <returns>The offset to add to every coordinate on the axis.</returns>
+        private static int ShiftInto(int minimum, int headroom)
+        {
+            int required = MinimumCoordinate - minimum;
+            if (required > headroom)
+            {
+                return Math.Max(required, 0);
+            }
+
+            return Math.Min(Math.Max(required, 0), headroom);
+        }
     }
 }

test/ThreatModelForge.Analysis.Tests/DeclarativeRuleProviderTests.cs

@@ -312,9 +312,9 @@ public void GeneralizedThreatMetadataFlowsThroughEveryAnalysisSurface()
             Assert.AreEqual("medical-device/privacy", exportedType.Category);
             Assert.AreEqual("High", exportedType.PropertiesMetaData.Single().Values.Single());
             Assert.IsTrue(knowledgeBase.ThreatMetaData!.IsPriorityUsed);
-            CollectionAssert.AreEqual(
-                new[] { "Critical", "High", "Medium", "Low" },
-                knowledgeBase.ThreatMetaData.PropertiesMetaData.Single().Values);
+            ThreatMetaDatum exportedPriority = knowledgeBase.ThreatMetaData.PropertiesMetaData
+                .Single(datum => datum.Name == "Priority");
+            CollectionAssert.AreEqual(new[] { "Critical", "High", "Medium", "Low" }, exportedPriority.Values);
         }
 
         /// <summary>Default threat priority must be valid and attached to a threat-bearing rule.</summary>