From 02031aee636cb10c1613c662cb638200f0d46534 Mon Sep 17 00:00:00 2001 From: Hacks4Snacks Date: Tue, 7 Jul 2026 23:12:46 -0500 Subject: [PATCH 1/8] threat generation support --- docs/cli-reference.md | 56 +++ docs/installation.md | 4 +- docs/validation-rules.md | 2 +- .../CachedCredentialReadRule.cs | 9 + .../CleartextTrustBoundaryCrossingRule.cs | 10 + .../CredentialsInLogStoreRule.cs | 9 + .../EdgeMissingDataClassificationRule.cs | 9 + .../EdgeMissingProtocolRule.cs | 9 + .../OverPrivilegedProcessRule.cs | 10 + .../RuleDocumentation.cs | 6 +- .../SharedStaticIdentityRule.cs | 9 + .../UnauthenticatedBoundaryProcessRule.cs | 10 + .../UnauthenticatedExternalSourceRule.cs | 10 + .../UnencryptedSecretStoreRule.cs | 9 + .../UnprotectedCredentialStoreRule.cs | 9 + .../UnsanitizedCrossBoundaryInputRule.cs | 10 + .../UnsanitizedExternalOutputRule.cs | 10 + .../UnsignedAuditLogStoreRule.cs | 9 + .../WeakCipherRule.cs | 10 + .../WeakProcessIsolationRule.cs | 9 + src/ThreatModelForge.Analysis/ApplyResult.cs | 17 + .../FindingCollector.cs | 35 ++ .../GeneratedThreat.cs | 64 +++ .../GenerationResult.cs | 25 ++ .../IsExternalInit.cs | 13 + src/ThreatModelForge.Analysis/Rule.cs | 15 + .../StrideCategory.cs | 28 ++ .../ThreatGenerator.cs | 370 ++++++++++++++++++ .../ThreatReference.cs | 51 +++ src/ThreatModelForge.Cli/AcceptCommand.cs | 108 +++++ src/ThreatModelForge.Cli/CommandCatalog.cs | 2 + src/ThreatModelForge.Cli/ThreatsCommand.cs | 191 +++++++++ src/ThreatModelForge.Engine/EngineService.cs | 68 ++++ src/ThreatModelForge.Engine/ThreatDto.cs | 41 ++ .../ConformanceTests.cs | 29 ++ .../ThreatGeneratorTest.cs | 165 ++++++++ .../EngineGenerateThreatsTest.cs | 56 +++ .../ThreatsCommandTest.cs | 172 ++++++++ 38 files changed, 1661 insertions(+), 8 deletions(-) create mode 100644 src/ThreatModelForge.Analysis/ApplyResult.cs create mode 100644 src/ThreatModelForge.Analysis/FindingCollector.cs create mode 100644 src/ThreatModelForge.Analysis/GeneratedThreat.cs create mode 100644 src/ThreatModelForge.Analysis/GenerationResult.cs create mode 100644 src/ThreatModelForge.Analysis/IsExternalInit.cs create mode 100644 src/ThreatModelForge.Analysis/StrideCategory.cs create mode 100644 src/ThreatModelForge.Analysis/ThreatGenerator.cs create mode 100644 src/ThreatModelForge.Analysis/ThreatReference.cs create mode 100644 src/ThreatModelForge.Cli/AcceptCommand.cs create mode 100644 src/ThreatModelForge.Cli/ThreatsCommand.cs create mode 100644 src/ThreatModelForge.Engine/ThreatDto.cs create mode 100644 test/ThreatModelForge.Analysis.Rules.Tests/ThreatGeneratorTest.cs create mode 100644 test/ThreatModelForge.Api.Tests/EngineGenerateThreatsTest.cs create mode 100644 test/ThreatModelForge.Cli.Tests/ThreatsCommandTest.cs diff --git a/docs/cli-reference.md b/docs/cli-reference.md index c795fab..79d5fbe 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -44,6 +44,8 @@ tmforge [options] | [`page`](#page) | Author | List, add, rename, reorder, or remove pages (diagrams). | | [`layout`](#layout) | Author | Auto-lay-out the diagram (layered; no hand-placed coordinates). | | [`lint`](#lint) | Validate | Evaluate the rule set against a model. | +| [`threats`](#threats) | Analyze | Report threats — the persisted, triaged view of the validation findings (`--write` to persist). | +| [`accept`](#accept) | Analyze | Accept a generated threat's risk (records a justification). | | [`report`](#report) | Report | Generate a self-contained HTML report. | | [`convert`](#convert) | Convert | Convert a model between file formats. | | [`apply`](#apply) | Author | Build a model from a declarative JSON manifest (all-or-nothing). | @@ -492,6 +494,60 @@ tmforge lint payments.tm7 --suppressionFile suppressions.json --json > (disabled packs/rules) is honored automatically. Other formats use the full rule set or an > explicit `--ruleset`. +### `threats` + +Report the model's **STRIDE threats** — the persisted, triaged view of the validation findings. +Detection is entirely the rule set: `threats` runs the same rules as [`lint`](#lint), keeps the +findings from threat-bearing rules, and frames each as a STRIDE threat against its element or flow. +The difference from `lint` is **lifecycle** — where a finding is transient and gated, a threat is +persisted and triaged (`open -> mitigated -> accepted`). With `--write`, the threats are persisted into +the model's register, keyed so a re-run updates in place and never overwrites prior triage. + +```text +tmforge threats [--write] [--json] +``` + +| Option | Meaning | +| --- | --- | +| `--write` | Persist the threats into the model's register (preserves prior triage). | + +Each threat carries a STRIDE category, the rule's mitigation, and external references (CWE / CAPEC). +There is no separate threat catalog: **extend coverage the way detection is extended — add a rule to a +rule pack** (see [Validation rules](validation-rules.md)). Rules that represent structural or naming +hygiene (rather than a security weakness) are not reported as threats. + +```bash +tmforge threats payments.tm7 # report (read-only), same findings as lint +tmforge threats payments.tm7 --write # persist into the register +tmforge threats payments.tm7 --json +``` + +After `--write`, triage the register with [`list threats`](#list) and [`accept`](#accept). + +### `accept` + +Record **inline risk acceptance** for a generated threat. Acceptance is a threat *state*: the threat +moves to `NotApplicable` with your justification, stays visible in the register and report, and no +longer counts as open. Because it is scoped to a single threat (one pattern on one interaction), it +can never silently swallow an unrelated finding — unlike a blanket suppression. + +```text +tmforge accept --threat --reason [--json] +``` + +| Option | Meaning | +| --- | --- | +| `--threat ` | The threat to accept: its register key, interaction key, or numeric id (from `list threats`). | +| `--reason ` | The justification, stored with the threat and shown in the register and report. | + +```bash +tmforge threats payments.tm7 --write +tmforge list threats payments.tm7 +tmforge accept --threat 3 --reason "Mitigated by the upstream WAF; residual risk accepted." payments.tm7 +``` + +Acceptance round-trips through `.tm7` and is honored identically by the CLI, the `/v1` API, and Studio. + ### `report` Generate a self-contained HTML report with an inline SVG diagram per page, or export just the diff --git a/docs/installation.md b/docs/installation.md index aeffc6a..f6f0a02 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -30,7 +30,7 @@ are attached to each GitHub Release for six platforms: ```bash # Replace OWNER/REPO, the version, and the RID for your platform. curl -fsSL -o tmforge.tar.gz \ - https://github.com/OWNER/REPO/releases/download/v0.1.0/tmforge-0.1.0-linux-x64.tar.gz + https://github.com/hacks4snacks/tmforge/releases/download/v0.1.0/tmforge-0.1.0-linux-x64.tar.gz tar -xzf tmforge.tar.gz ./tmforge-0.1.0-linux-x64/tmforge --version ``` @@ -41,7 +41,7 @@ Move the binary somewhere on your `PATH`, e.g. `sudo mv tmforge-0.1.0-linux-x64/ ```powershell Invoke-WebRequest -Uri ` - https://github.com/OWNER/REPO/releases/download/v0.1.0/tmforge-0.1.0-win-x64.zip ` + https://github.com/hacks4snacks/tmforge/releases/download/v0.1.0/tmforge-0.1.0-win-x64.zip ` -OutFile tmforge.zip Expand-Archive tmforge.zip -DestinationPath tmforge .\tmforge\tmforge.exe --version diff --git a/docs/validation-rules.md b/docs/validation-rules.md index a6c046a..d6da2f8 100644 --- a/docs/validation-rules.md +++ b/docs/validation-rules.md @@ -200,7 +200,7 @@ jobs: - name: Download tmforge run: | curl -fsSL -o tmforge.tar.gz \ - https://github.com/OWNER/REPO/releases/download/v0.1.0/tmforge-0.1.0-linux-x64.tar.gz + https://github.com/hacks4snacks/tmforge/releases/download/v0.1.0/tmforge-0.1.0-linux-x64.tar.gz tar -xzf tmforge.tar.gz echo "$PWD/tmforge-0.1.0-linux-x64" >> "$GITHUB_PATH" - name: Validate threat models diff --git a/src/ThreatModelForge.Analysis.Rules/CachedCredentialReadRule.cs b/src/ThreatModelForge.Analysis.Rules/CachedCredentialReadRule.cs index 4d7e783..f311118 100644 --- a/src/ThreatModelForge.Analysis.Rules/CachedCredentialReadRule.cs +++ b/src/ThreatModelForge.Analysis.Rules/CachedCredentialReadRule.cs @@ -33,6 +33,15 @@ public CachedCredentialReadRule() new PropertyBinding("flow", CachedCustomAttributeName, "Yes"), }; + /// + public override StrideCategory? Stride => StrideCategory.InformationDisclosure; + + /// + public override IReadOnlyList ThreatReferences => new[] + { + ThreatReference.Cwe(522), + }; + /// public override void Evaluate(RuleEvaluationContext context) { diff --git a/src/ThreatModelForge.Analysis.Rules/CleartextTrustBoundaryCrossingRule.cs b/src/ThreatModelForge.Analysis.Rules/CleartextTrustBoundaryCrossingRule.cs index feaf9ac..8d2ead0 100644 --- a/src/ThreatModelForge.Analysis.Rules/CleartextTrustBoundaryCrossingRule.cs +++ b/src/ThreatModelForge.Analysis.Rules/CleartextTrustBoundaryCrossingRule.cs @@ -51,6 +51,16 @@ public CleartextTrustBoundaryCrossingRule() new PropertyBinding("flow", "Protocol", "HTTP", "FTP"), }; + /// + public override StrideCategory? Stride => StrideCategory.InformationDisclosure; + + /// + public override IReadOnlyList ThreatReferences => new[] + { + ThreatReference.Cwe(319), + ThreatReference.Capec(157), + }; + /// public override void Evaluate(RuleEvaluationContext context) { diff --git a/src/ThreatModelForge.Analysis.Rules/CredentialsInLogStoreRule.cs b/src/ThreatModelForge.Analysis.Rules/CredentialsInLogStoreRule.cs index f7a6429..f3ced3c 100644 --- a/src/ThreatModelForge.Analysis.Rules/CredentialsInLogStoreRule.cs +++ b/src/ThreatModelForge.Analysis.Rules/CredentialsInLogStoreRule.cs @@ -32,6 +32,15 @@ public CredentialsInLogStoreRule() new PropertyBinding("datastore", "StoresCredentials"), }; + /// + public override StrideCategory? Stride => StrideCategory.InformationDisclosure; + + /// + public override IReadOnlyList ThreatReferences => new[] + { + ThreatReference.Cwe(532), + }; + /// public override void Evaluate(RuleEvaluationContext context) { diff --git a/src/ThreatModelForge.Analysis.Rules/EdgeMissingDataClassificationRule.cs b/src/ThreatModelForge.Analysis.Rules/EdgeMissingDataClassificationRule.cs index 2553cb8..8e84c01 100644 --- a/src/ThreatModelForge.Analysis.Rules/EdgeMissingDataClassificationRule.cs +++ b/src/ThreatModelForge.Analysis.Rules/EdgeMissingDataClassificationRule.cs @@ -25,6 +25,15 @@ public EdgeMissingDataClassificationRule() new PropertyBinding("flow", "DataType"), }; + /// + public override StrideCategory? Stride => StrideCategory.InformationDisclosure; + + /// + public override IReadOnlyList ThreatReferences => new[] + { + ThreatReference.Cwe(200), + }; + /// public override void Evaluate(RuleEvaluationContext context) { diff --git a/src/ThreatModelForge.Analysis.Rules/EdgeMissingProtocolRule.cs b/src/ThreatModelForge.Analysis.Rules/EdgeMissingProtocolRule.cs index 0b7ba99..c2454c0 100644 --- a/src/ThreatModelForge.Analysis.Rules/EdgeMissingProtocolRule.cs +++ b/src/ThreatModelForge.Analysis.Rules/EdgeMissingProtocolRule.cs @@ -25,6 +25,15 @@ public EdgeMissingProtocolRule() new PropertyBinding("flow", "Protocol"), }; + /// + public override StrideCategory? Stride => StrideCategory.Tampering; + + /// + public override IReadOnlyList ThreatReferences => new[] + { + ThreatReference.Cwe(319), + }; + /// public override void Evaluate(RuleEvaluationContext context) { diff --git a/src/ThreatModelForge.Analysis.Rules/OverPrivilegedProcessRule.cs b/src/ThreatModelForge.Analysis.Rules/OverPrivilegedProcessRule.cs index 0d1b208..1b6aae2 100644 --- a/src/ThreatModelForge.Analysis.Rules/OverPrivilegedProcessRule.cs +++ b/src/ThreatModelForge.Analysis.Rules/OverPrivilegedProcessRule.cs @@ -31,6 +31,16 @@ public OverPrivilegedProcessRule() new PropertyBinding("process", "RunningAs", "System", "Root/Admin"), }; + /// + public override StrideCategory? Stride => StrideCategory.ElevationOfPrivilege; + + /// + public override IReadOnlyList ThreatReferences => new[] + { + ThreatReference.Cwe(250), + ThreatReference.Capec(233), + }; + /// public override void Evaluate(RuleEvaluationContext context) { diff --git a/src/ThreatModelForge.Analysis.Rules/RuleDocumentation.cs b/src/ThreatModelForge.Analysis.Rules/RuleDocumentation.cs index 6313a4d..861e428 100644 --- a/src/ThreatModelForge.Analysis.Rules/RuleDocumentation.cs +++ b/src/ThreatModelForge.Analysis.Rules/RuleDocumentation.cs @@ -25,12 +25,8 @@ public static class RuleDocumentation /// /// The canonical, public documentation page for the built-in rule set. /// - /// - /// OWNER/REPO is a placeholder until the public repository is published; replace it - /// here to activate real help links across every surface. - /// public const string RulesReferenceUrl = - "https://github.com/OWNER/REPO/blob/main/docs/validation-rules.md"; + "https://github.com/hacks4snacks/tmforge/blob/main/docs/validation-rules.md"; /// /// Builds the documentation URL for a single rule. diff --git a/src/ThreatModelForge.Analysis.Rules/SharedStaticIdentityRule.cs b/src/ThreatModelForge.Analysis.Rules/SharedStaticIdentityRule.cs index 44c6425..8fbe40d 100644 --- a/src/ThreatModelForge.Analysis.Rules/SharedStaticIdentityRule.cs +++ b/src/ThreatModelForge.Analysis.Rules/SharedStaticIdentityRule.cs @@ -32,6 +32,15 @@ public SharedStaticIdentityRule() new PropertyBinding("flow", IdentityCustomAttributeName), }; + /// + public override StrideCategory? Stride => StrideCategory.Spoofing; + + /// + public override IReadOnlyList ThreatReferences => new[] + { + ThreatReference.Cwe(287), + }; + /// public override void Evaluate(RuleEvaluationContext context) { diff --git a/src/ThreatModelForge.Analysis.Rules/UnauthenticatedBoundaryProcessRule.cs b/src/ThreatModelForge.Analysis.Rules/UnauthenticatedBoundaryProcessRule.cs index 997b343..6c35249 100644 --- a/src/ThreatModelForge.Analysis.Rules/UnauthenticatedBoundaryProcessRule.cs +++ b/src/ThreatModelForge.Analysis.Rules/UnauthenticatedBoundaryProcessRule.cs @@ -33,6 +33,16 @@ public UnauthenticatedBoundaryProcessRule() new PropertyBinding("process", "AuthenticationScheme", "None"), }; + /// + public override StrideCategory? Stride => StrideCategory.Spoofing; + + /// + public override IReadOnlyList ThreatReferences => new[] + { + ThreatReference.Cwe(306), + ThreatReference.Capec(115), + }; + /// public override void Evaluate(RuleEvaluationContext context) { diff --git a/src/ThreatModelForge.Analysis.Rules/UnauthenticatedExternalSourceRule.cs b/src/ThreatModelForge.Analysis.Rules/UnauthenticatedExternalSourceRule.cs index f6aa83f..7104b2b 100644 --- a/src/ThreatModelForge.Analysis.Rules/UnauthenticatedExternalSourceRule.cs +++ b/src/ThreatModelForge.Analysis.Rules/UnauthenticatedExternalSourceRule.cs @@ -36,6 +36,16 @@ public UnauthenticatedExternalSourceRule() new PropertyBinding("external", "AuthenticationScheme", "None"), }; + /// + public override StrideCategory? Stride => StrideCategory.Spoofing; + + /// + public override IReadOnlyList ThreatReferences => new[] + { + ThreatReference.Cwe(287), + ThreatReference.Capec(151), + }; + /// public override void Evaluate(RuleEvaluationContext context) { diff --git a/src/ThreatModelForge.Analysis.Rules/UnencryptedSecretStoreRule.cs b/src/ThreatModelForge.Analysis.Rules/UnencryptedSecretStoreRule.cs index 6569527..6da9c17 100644 --- a/src/ThreatModelForge.Analysis.Rules/UnencryptedSecretStoreRule.cs +++ b/src/ThreatModelForge.Analysis.Rules/UnencryptedSecretStoreRule.cs @@ -32,6 +32,15 @@ public UnencryptedSecretStoreRule() new PropertyBinding("datastore", "StoresCredentials"), }; + /// + public override StrideCategory? Stride => StrideCategory.InformationDisclosure; + + /// + public override IReadOnlyList ThreatReferences => new[] + { + ThreatReference.Cwe(311), + }; + /// public override void Evaluate(RuleEvaluationContext context) { diff --git a/src/ThreatModelForge.Analysis.Rules/UnprotectedCredentialStoreRule.cs b/src/ThreatModelForge.Analysis.Rules/UnprotectedCredentialStoreRule.cs index 5a4209c..eb192e0 100644 --- a/src/ThreatModelForge.Analysis.Rules/UnprotectedCredentialStoreRule.cs +++ b/src/ThreatModelForge.Analysis.Rules/UnprotectedCredentialStoreRule.cs @@ -32,6 +32,15 @@ public UnprotectedCredentialStoreRule() new PropertyBinding("datastore", "StoresCredentials"), }; + /// + public override StrideCategory? Stride => StrideCategory.InformationDisclosure; + + /// + public override IReadOnlyList ThreatReferences => new[] + { + ThreatReference.Cwe(284), + }; + /// public override void Evaluate(RuleEvaluationContext context) { diff --git a/src/ThreatModelForge.Analysis.Rules/UnsanitizedCrossBoundaryInputRule.cs b/src/ThreatModelForge.Analysis.Rules/UnsanitizedCrossBoundaryInputRule.cs index 41b58b1..a44d90c 100644 --- a/src/ThreatModelForge.Analysis.Rules/UnsanitizedCrossBoundaryInputRule.cs +++ b/src/ThreatModelForge.Analysis.Rules/UnsanitizedCrossBoundaryInputRule.cs @@ -32,6 +32,16 @@ public UnsanitizedCrossBoundaryInputRule() new PropertyBinding("process", "SanitizesInput", "No"), }; + /// + public override StrideCategory? Stride => StrideCategory.Tampering; + + /// + public override IReadOnlyList ThreatReferences => new[] + { + ThreatReference.Cwe(20), + ThreatReference.Capec(66), + }; + /// public override void Evaluate(RuleEvaluationContext context) { diff --git a/src/ThreatModelForge.Analysis.Rules/UnsanitizedExternalOutputRule.cs b/src/ThreatModelForge.Analysis.Rules/UnsanitizedExternalOutputRule.cs index e9a3f8e..75ae370 100644 --- a/src/ThreatModelForge.Analysis.Rules/UnsanitizedExternalOutputRule.cs +++ b/src/ThreatModelForge.Analysis.Rules/UnsanitizedExternalOutputRule.cs @@ -32,6 +32,16 @@ public UnsanitizedExternalOutputRule() new PropertyBinding("process", "SanitizesOutput", "No"), }; + /// + public override StrideCategory? Stride => StrideCategory.Tampering; + + /// + public override IReadOnlyList ThreatReferences => new[] + { + ThreatReference.Cwe(116), + ThreatReference.Capec(63), + }; + /// public override void Evaluate(RuleEvaluationContext context) { diff --git a/src/ThreatModelForge.Analysis.Rules/UnsignedAuditLogStoreRule.cs b/src/ThreatModelForge.Analysis.Rules/UnsignedAuditLogStoreRule.cs index 28f32c6..42a1a8b 100644 --- a/src/ThreatModelForge.Analysis.Rules/UnsignedAuditLogStoreRule.cs +++ b/src/ThreatModelForge.Analysis.Rules/UnsignedAuditLogStoreRule.cs @@ -32,6 +32,15 @@ public UnsignedAuditLogStoreRule() new PropertyBinding("datastore", "StoresLogData"), }; + /// + public override StrideCategory? Stride => StrideCategory.Repudiation; + + /// + public override IReadOnlyList ThreatReferences => new[] + { + ThreatReference.Cwe(778), + }; + /// public override void Evaluate(RuleEvaluationContext context) { diff --git a/src/ThreatModelForge.Analysis.Rules/WeakCipherRule.cs b/src/ThreatModelForge.Analysis.Rules/WeakCipherRule.cs index 3250849..a35f383 100644 --- a/src/ThreatModelForge.Analysis.Rules/WeakCipherRule.cs +++ b/src/ThreatModelForge.Analysis.Rules/WeakCipherRule.cs @@ -47,6 +47,16 @@ public WeakCipherRule() new PropertyBinding("flow", AlgorithmCustomAttributeName, "AES-CBC", "3DES", "RC4", "secretbox", "Other"), }; + /// + public override StrideCategory? Stride => StrideCategory.InformationDisclosure; + + /// + public override IReadOnlyList ThreatReferences => new[] + { + ThreatReference.Cwe(327), + ThreatReference.Capec(97), + }; + /// public override void Evaluate(RuleEvaluationContext context) { diff --git a/src/ThreatModelForge.Analysis.Rules/WeakProcessIsolationRule.cs b/src/ThreatModelForge.Analysis.Rules/WeakProcessIsolationRule.cs index e890e16..e579b6d 100644 --- a/src/ThreatModelForge.Analysis.Rules/WeakProcessIsolationRule.cs +++ b/src/ThreatModelForge.Analysis.Rules/WeakProcessIsolationRule.cs @@ -32,6 +32,15 @@ public WeakProcessIsolationRule() new PropertyBinding("process", "Isolation", "None"), }; + /// + public override StrideCategory? Stride => StrideCategory.ElevationOfPrivilege; + + /// + public override IReadOnlyList ThreatReferences => new[] + { + ThreatReference.Cwe(693), + }; + /// public override void Evaluate(RuleEvaluationContext context) { diff --git a/src/ThreatModelForge.Analysis/ApplyResult.cs b/src/ThreatModelForge.Analysis/ApplyResult.cs new file mode 100644 index 0000000..4a37376 --- /dev/null +++ b/src/ThreatModelForge.Analysis/ApplyResult.cs @@ -0,0 +1,17 @@ +namespace ThreatModelForge.Analysis +{ + /// + /// A summary of an idempotent pass over a model's register. + /// + public sealed class ApplyResult + { + /// Gets the number of new threats added to the register. + public int Added { get; init; } + + /// Gets the number of existing auto-generated threats refreshed in place. + public int Updated { get; init; } + + /// Gets the number of human-triaged threats left untouched. + public int Preserved { get; init; } + } +} diff --git a/src/ThreatModelForge.Analysis/FindingCollector.cs b/src/ThreatModelForge.Analysis/FindingCollector.cs new file mode 100644 index 0000000..f15bcf5 --- /dev/null +++ b/src/ThreatModelForge.Analysis/FindingCollector.cs @@ -0,0 +1,35 @@ +namespace ThreatModelForge.Analysis +{ + using System; + using System.Collections.Generic; + + /// + /// A that captures the raw rule findings — each with its source rule + /// and target element/flow — so the threat projection can key every finding to the element or flow + /// it was raised against. This is how validation findings inform the threat register. + /// + internal sealed class FindingCollector : MessageWriter + { + private readonly List messages = new List(); + + /// Gets the collected findings. + public IReadOnlyList Messages => this.messages; + + /// + public override void Write(Message message) + { + if (message == null) + { + throw new ArgumentNullException(nameof(message)); + } + + this.messages.Add(message); + } + + /// + public override void WriteCore(MessageSeverity severity, string messageID, string text) + { + // The projection reads whole Message objects via Write; the flat overload is unused. + } + } +} diff --git a/src/ThreatModelForge.Analysis/GeneratedThreat.cs b/src/ThreatModelForge.Analysis/GeneratedThreat.cs new file mode 100644 index 0000000..1b55190 --- /dev/null +++ b/src/ThreatModelForge.Analysis/GeneratedThreat.cs @@ -0,0 +1,64 @@ +namespace ThreatModelForge.Analysis +{ + using System; + using System.Collections.Generic; + + /// + /// A threat projected from a validation finding: the rule that detected it, its STRIDE category and + /// external references, and the element or flow it was raised against. A generated threat is simply + /// the persistable, lifecycle-bearing form of a finding — the detection is entirely the rule's. + /// + public sealed class GeneratedThreat + { + /// Gets the deterministic identifier of the threat ({targetGuid:N}:{ruleId}). + public string Id { get; init; } = string.Empty; + + /// Gets the identifier of the rule that detected the threat (for example TM1023). + public string RuleId { get; init; } = string.Empty; + + /// Gets the STRIDE category. + public StrideCategory Category { get; init; } + + /// Gets the threat title (the finding text). + public string Title { get; init; } = string.Empty; + + /// Gets the suggested mitigation (the rule's help text). + public string? Mitigation { get; init; } + + /// Gets the rule severity (error, warning, or info). + public string Severity { get; init; } = "warning"; + + /// Gets the coarse priority derived from severity (High / Medium / Low). + public string Priority { get; init; } = "Medium"; + + /// Gets the external catalog references. + public IReadOnlyList References { get; init; } = Array.Empty(); + + /// Gets the source element identifier (the target element for an element-scoped finding). + public Guid SourceGuid { get; init; } + + /// Gets the target element identifier (empty for an element-scoped finding). + public Guid TargetGuid { get; init; } + + /// Gets the flow identifier (empty for an element-scoped finding). + public Guid FlowGuid { get; init; } + + /// Gets the diagram identifier. + public Guid DiagramGuid { get; init; } + + /// Gets the source element display name. + public string? SourceName { get; init; } + + /// Gets the target element display name. + public string? TargetName { get; init; } + + /// Gets the flow display name. + public string? FlowName { get; init; } + + /// Gets a value indicating whether the threat is scoped to a flow (an interaction) rather than a single element. + public bool IsFlowScoped { get; init; } + + /// Gets the human-readable scope string (source -> target for a flow, else the element name). + public string InteractionString { get; init; } = string.Empty; + } +} diff --git a/src/ThreatModelForge.Analysis/GenerationResult.cs b/src/ThreatModelForge.Analysis/GenerationResult.cs new file mode 100644 index 0000000..60269c1 --- /dev/null +++ b/src/ThreatModelForge.Analysis/GenerationResult.cs @@ -0,0 +1,25 @@ +namespace ThreatModelForge.Analysis +{ + using System; + using System.Collections.Generic; + + /// + /// The outcome of a threat-generation pass: every threat projected from the model's validation + /// findings. + /// + public sealed class GenerationResult + { + /// Initializes a new instance of the class. + /// The generated threats. + public GenerationResult(IReadOnlyList threats) + { + this.Threats = threats ?? Array.Empty(); + } + + /// Gets every generated threat. + public IReadOnlyList Threats { get; } + + /// Gets the number of generated threats. + public int Count => this.Threats.Count; + } +} diff --git a/src/ThreatModelForge.Analysis/IsExternalInit.cs b/src/ThreatModelForge.Analysis/IsExternalInit.cs new file mode 100644 index 0000000..d2cc013 --- /dev/null +++ b/src/ThreatModelForge.Analysis/IsExternalInit.cs @@ -0,0 +1,13 @@ +namespace System.Runtime.CompilerServices +{ + using System.ComponentModel; + + /// + /// Enables C# init-only property accessors on netstandard2.0, which does not ship + /// this compiler-support type in-box. Referenced only by the compiler; never used at runtime. + /// + [EditorBrowsable(EditorBrowsableState.Never)] + internal static class IsExternalInit + { + } +} diff --git a/src/ThreatModelForge.Analysis/Rule.cs b/src/ThreatModelForge.Analysis/Rule.cs index 3faf4de..959fab6 100644 --- a/src/ThreatModelForge.Analysis/Rule.cs +++ b/src/ThreatModelForge.Analysis/Rule.cs @@ -91,6 +91,21 @@ public string ID /// public virtual IReadOnlyList PropertyBindings => Array.Empty(); + /// + /// Gets the STRIDE category the rule's finding represents, or when the + /// rule is a structural or naming hygiene check rather than a threat. Threat generation reads + /// this to decide which findings become persisted threats, so a rule declares its own threat + /// identity in one place and there is no separate map to drift from. + /// + public virtual StrideCategory? Stride => null; + + /// + /// Gets the external catalog references (CWE / CAPEC / MITRE ATT&CK) for the threat this + /// rule detects. Declared together with on a threat-bearing rule; empty by + /// default. The mitigation is not duplicated here — it is the rule's . + /// + public virtual IReadOnlyList ThreatReferences => Array.Empty(); + /// public void Dispose() { diff --git a/src/ThreatModelForge.Analysis/StrideCategory.cs b/src/ThreatModelForge.Analysis/StrideCategory.cs new file mode 100644 index 0000000..a517447 --- /dev/null +++ b/src/ThreatModelForge.Analysis/StrideCategory.cs @@ -0,0 +1,28 @@ +namespace ThreatModelForge.Analysis +{ + /// + /// The six STRIDE threat categories. A may declare the category its finding + /// represents (see ); a rule that leaves it unset is a structural or + /// naming hygiene check, not a threat. + /// + public enum StrideCategory + { + /// Impersonating something or someone else. + Spoofing, + + /// Unauthorized modification of data or code. + Tampering, + + /// Denying an action without other parties being able to prove otherwise. + Repudiation, + + /// Exposure of information to parties not authorized to see it. + InformationDisclosure, + + /// Denying or degrading service to legitimate users. + DenialOfService, + + /// Gaining capabilities without proper authorization. + ElevationOfPrivilege, + } +} diff --git a/src/ThreatModelForge.Analysis/ThreatGenerator.cs b/src/ThreatModelForge.Analysis/ThreatGenerator.cs new file mode 100644 index 0000000..d1539b1 --- /dev/null +++ b/src/ThreatModelForge.Analysis/ThreatGenerator.cs @@ -0,0 +1,370 @@ +namespace ThreatModelForge.Analysis +{ + using System; + using System.Collections.Generic; + using System.Globalization; + using System.Linq; + using System.Reflection; + using ThreatModelForge.KnowledgeBase; + using ThreatModelForge.Model; + using ThreatModelForge.Model.Abstracts; + + /// + /// Projects the validation findings of a model into its persistent threat register. Detection is + /// entirely the rule set's job — this class does not re-implement any condition. It runs + /// the same rules that lint runs, keeps only the findings from threat-bearing rules (those + /// whose rule declares a STRIDE category), frames each as a STRIDE threat against its element or + /// flow, and upserts it into the model with an idempotent, triage-preserving write. The difference + /// between a finding and a threat is lifecycle: a finding is transient and gated; a threat is + /// persisted and triaged (open -> mitigated -> accepted). + /// + public static class ThreatGenerator + { + /// Generates threats by running the default rule set over the model. + /// The model to analyze. + /// The generated threats. + public static GenerationResult Generate(ThreatModel model) + { + using (RuleSet ruleSet = LoadDefaultRuleSet()) + { + return Generate(model, ruleSet); + } + } + + /// Generates threats by projecting the given rule set's findings over the model. + /// The model to analyze. + /// The rule set whose findings are projected (the detection source). + /// The generated threats. + public static GenerationResult Generate(ThreatModel model, RuleSet ruleSet) + { + if (model == null) + { + throw new ArgumentNullException(nameof(model)); + } + + if (ruleSet == null) + { + throw new ArgumentNullException(nameof(ruleSet)); + } + + FindingCollector collector = new FindingCollector(); + RuleEvaluationContext context = new RuleEvaluationContext(model, collector); + ruleSet.Evaluate(context); + + List threats = new List(); + HashSet seen = new HashSet(StringComparer.OrdinalIgnoreCase); + foreach (Message message in collector.Messages) + { + Rule? rule = message.Source; + Entity? target = message.Target; + if (rule == null || target == null) + { + continue; + } + + if (rule.Stride is not StrideCategory category) + { + continue; + } + + GeneratedThreat threat = ToThreat(message, rule, target, category); + if (seen.Add(threat.Id)) + { + threats.Add(threat); + } + } + + threats.Sort(CompareThreats); + return new GenerationResult(threats); + } + + /// + /// Idempotently writes the generated threats into the model's register. Threats are keyed by + /// {targetGuid:N}:{ruleId}, so re-running updates in place: a human-triaged threat (any + /// state other than , including an accepted risk) is + /// preserved untouched, an existing auto-generated threat is refreshed, and a newly-detected + /// threat is added. Nothing is deleted, so no triage is ever lost. + /// + /// The model whose register is updated. + /// The generation result to apply. + /// A summary of the changes made. + public static ApplyResult Apply(ThreatModel model, GenerationResult result) + { + if (model == null) + { + throw new ArgumentNullException(nameof(model)); + } + + if (result == null) + { + throw new ArgumentNullException(nameof(result)); + } + + int added = 0; + int updated = 0; + int preserved = 0; + int nextId = NextThreatId(model); + foreach (GeneratedThreat threat in result.Threats) + { + if (model.AllThreatsDictionary.TryGetValue(threat.Id, out Threat? existing)) + { + if (existing.State != ThreatState.AutoGenerated) + { + preserved++; + continue; + } + + UpdateThreat(existing, threat); + updated++; + } + else + { + model.AllThreatsDictionary[threat.Id] = CreateThreat(threat, nextId); + nextId++; + added++; + } + } + + model.ThreatGenerationEnabled = true; + return new ApplyResult { Added = added, Updated = updated, Preserved = preserved }; + } + + /// + /// Records risk acceptance for a threat: moves it to + /// and stores the justification. The threat stays visible in the register and report but is no + /// longer treated as open. Because acceptance is per-threat (one rule finding on one element or + /// flow), it can never silently swallow an unrelated finding. + /// + /// The model containing the threat. + /// The threat identifier (its register key, interaction key, or numeric id). + /// The justification for accepting the risk. + /// if a matching threat was found and accepted; otherwise . + public static bool Accept(ThreatModel model, string threatId, string reason) + { + if (model == null) + { + throw new ArgumentNullException(nameof(model)); + } + + if (string.IsNullOrWhiteSpace(threatId)) + { + throw new ArgumentException("Value cannot be null or empty.", nameof(threatId)); + } + + Threat? threat = FindThreat(model, threatId); + if (threat == null) + { + return false; + } + + threat.State = ThreatState.NotApplicable; + threat.StateInformation = reason ?? string.Empty; + threat.ModifiedAt = DateTime.UtcNow; + return true; + } + + private static RuleSet LoadDefaultRuleSet() + { + Assembly rules = Assembly.Load("ThreatModelForge.Analysis.Rules"); + return RuleSet.LoadDefault(new[] { rules }); + } + + private static GeneratedThreat ToThreat(Message message, Rule rule, Entity target, StrideCategory category) + { + bool isFlow = target is Connector; + Guid sourceGuid = target.Guid; + Guid targetGuid = Guid.Empty; + Guid flowGuid = Guid.Empty; + string? sourceName = NameOf(target); + string? targetName = null; + string? flowName = null; + string interactionString = sourceName ?? target.Guid.ToString(); + + if (target is Connector connector) + { + sourceGuid = connector.SourceGuid; + targetGuid = connector.TargetGuid; + flowGuid = connector.Guid; + flowName = NameOf(connector); + sourceName = NameInDiagram(message.Model, connector.SourceGuid); + targetName = NameInDiagram(message.Model, connector.TargetGuid); + interactionString = (sourceName ?? "?") + " -> " + (targetName ?? "?"); + } + + return new GeneratedThreat + { + Id = string.Format(CultureInfo.InvariantCulture, "{0:N}:{1}", target.Guid, rule.ID), + RuleId = rule.ID, + Category = category, + Title = message.Text ?? rule.FullDescription, + Mitigation = string.IsNullOrWhiteSpace(rule.HelpText) ? null : rule.HelpText, + Severity = rule.Severity.ToString().ToLowerInvariant(), + Priority = PriorityFor(rule.Severity), + References = rule.ThreatReferences, + SourceGuid = sourceGuid, + TargetGuid = targetGuid, + FlowGuid = flowGuid, + DiagramGuid = message.Model?.Guid ?? Guid.Empty, + SourceName = sourceName, + TargetName = targetName, + FlowName = flowName, + IsFlowScoped = isFlow, + InteractionString = interactionString, + }; + } + + private static Threat CreateThreat(GeneratedThreat threat, int id) + { + return new Threat + { + Id = id, + TypeId = threat.RuleId, + SourceGuid = threat.SourceGuid, + TargetGuid = threat.TargetGuid, + FlowGuid = threat.FlowGuid, + DrawingSurfaceGuid = threat.DiagramGuid, + State = ThreatState.AutoGenerated, + InteractionKey = threat.Id, + InteractionString = threat.InteractionString, + Title = threat.Title, + Priority = threat.Priority, + UserThreatCategory = CategoryName(threat.Category), + StateInformation = string.Empty, + ModifiedAt = DateTime.UtcNow, + Properties = BuildProperties(threat), + }; + } + + private static void UpdateThreat(Threat existing, GeneratedThreat threat) + { + existing.TypeId = threat.RuleId; + existing.SourceGuid = threat.SourceGuid; + existing.TargetGuid = threat.TargetGuid; + existing.FlowGuid = threat.FlowGuid; + existing.DrawingSurfaceGuid = threat.DiagramGuid; + existing.InteractionKey = threat.Id; + existing.InteractionString = threat.InteractionString; + existing.Title = threat.Title; + existing.Priority = threat.Priority; + existing.UserThreatCategory = CategoryName(threat.Category); + existing.Properties = BuildProperties(threat); + existing.ModifiedAt = DateTime.UtcNow; + } + + private static Dictionary BuildProperties(GeneratedThreat threat) + { + Dictionary props = new Dictionary(StringComparer.OrdinalIgnoreCase) + { + ["Rule"] = threat.RuleId, + }; + if (!string.IsNullOrWhiteSpace(threat.Mitigation)) + { + props["Mitigation"] = threat.Mitigation!; + } + + if (threat.References.Count > 0) + { + props["References"] = string.Join(", ", threat.References.Select(r => r.Id)); + } + + return props; + } + + private static string PriorityFor(MessageSeverity severity) + { + switch (severity) + { + case MessageSeverity.Error: + return "High"; + case MessageSeverity.Info: + return "Low"; + default: + return "Medium"; + } + } + + private static string CategoryName(StrideCategory category) + { + switch (category) + { + case StrideCategory.InformationDisclosure: + return "Information Disclosure"; + case StrideCategory.DenialOfService: + return "Denial of Service"; + case StrideCategory.ElevationOfPrivilege: + return "Elevation of Privilege"; + default: + return category.ToString(); + } + } + + private static int NextThreatId(ThreatModel model) + { + int max = 0; + foreach (Threat threat in model.AllThreatsDictionary.Values) + { + if (threat.Id > max) + { + max = threat.Id; + } + } + + return max + 1; + } + + private static Threat? FindThreat(ThreatModel model, string threatId) + { + if (model.AllThreatsDictionary.TryGetValue(threatId, out Threat? byKey)) + { + return byKey; + } + + foreach (KeyValuePair pair in model.AllThreatsDictionary) + { + if (string.Equals(pair.Value.InteractionKey, threatId, StringComparison.OrdinalIgnoreCase)) + { + return pair.Value; + } + + if (string.Equals(pair.Value.Id.ToString(CultureInfo.InvariantCulture), threatId, StringComparison.Ordinal)) + { + return pair.Value; + } + } + + return null; + } + + private static string? NameInDiagram(DrawingSurfaceModel? diagram, Guid id) + { + if (diagram != null && diagram.Borders.TryGetValue(id, out object? value) && value is Entity entity) + { + return NameOf(entity); + } + + return null; + } + + private static string? NameOf(Entity entity) + { + return entity.Name() ?? entity.HeaderName(); + } + + private static int CompareThreats(GeneratedThreat left, GeneratedThreat right) + { + int byCategory = left.Category.CompareTo(right.Category); + if (byCategory != 0) + { + return byCategory; + } + + int byRule = string.CompareOrdinal(left.RuleId, right.RuleId); + if (byRule != 0) + { + return byRule; + } + + return string.CompareOrdinal(left.Id, right.Id); + } + } +} diff --git a/src/ThreatModelForge.Analysis/ThreatReference.cs b/src/ThreatModelForge.Analysis/ThreatReference.cs new file mode 100644 index 0000000..65415f1 --- /dev/null +++ b/src/ThreatModelForge.Analysis/ThreatReference.cs @@ -0,0 +1,51 @@ +namespace ThreatModelForge.Analysis +{ + using System; + + /// + /// A first-class reference from a (and the threat projected from its finding) to + /// an external catalog entry — CWE, CAPEC, or MITRE ATT&CK. References are typed rather than + /// free text because external catalogs are a graph of cross-referenced identifiers. Construct with + /// , , or . + /// + public sealed class ThreatReference + { + /// Initializes a new instance of the class. + /// The catalog (for example CWE, CAPEC, ATTACK). + /// The identifier within the catalog (for example CWE-287). + /// An optional canonical URL for the referenced entry. + public ThreatReference(string catalog, string id, string? url = null) + { + this.Catalog = catalog ?? throw new ArgumentNullException(nameof(catalog)); + this.Id = id ?? throw new ArgumentNullException(nameof(id)); + this.Url = url; + } + + /// Gets the catalog the reference belongs to (for example CWE, CAPEC, ATTACK). + public string Catalog { get; } + + /// Gets the identifier within the catalog (for example CWE-287, CAPEC-151). + public string Id { get; } + + /// Gets an optional canonical URL for the referenced entry. + public string? Url { get; } + + /// Creates a CWE (Common Weakness Enumeration) reference. + /// The numeric CWE identifier. + /// A new reference. + public static ThreatReference Cwe(int id) => + new ThreatReference("CWE", $"CWE-{id}", $"https://cwe.mitre.org/data/definitions/{id}.html"); + + /// Creates a CAPEC (Common Attack Pattern Enumeration and Classification) reference. + /// The numeric CAPEC identifier. + /// A new reference. + public static ThreatReference Capec(int id) => + new ThreatReference("CAPEC", $"CAPEC-{id}", $"https://capec.mitre.org/data/definitions/{id}.html"); + + /// Creates a MITRE ATT&CK technique reference. + /// The technique identifier (for example T1190). + /// A new reference. + public static ThreatReference Attack(string techniqueId) => + new ThreatReference("ATTACK", techniqueId, $"https://attack.mitre.org/techniques/{techniqueId}/"); + } +} diff --git a/src/ThreatModelForge.Cli/AcceptCommand.cs b/src/ThreatModelForge.Cli/AcceptCommand.cs new file mode 100644 index 0000000..7ec8746 --- /dev/null +++ b/src/ThreatModelForge.Cli/AcceptCommand.cs @@ -0,0 +1,108 @@ +namespace ThreatModelForge.Cli +{ + using System; + using System.IO; + using ThreatModelForge.Analysis; + using ThreatModelForge.Formats; + using ThreatModelForge.Model; + + /// + /// Implements tmforge accept: records inline risk acceptance for a generated threat. + /// Acceptance is a threat state — the threat is moved to NotApplicable with a justification + /// and stays visible in the register and report, no longer counted as open. Because it is scoped to + /// one threat (a single pattern on a single interaction), it can never silently swallow an + /// unrelated finding. + /// + internal static class AcceptCommand + { + /// + /// Runs the accept command. + /// + /// The command arguments (after the verb). + /// Zero on success; a non-zero value on error. + public static int Run(string[] args) + { + if (args == null || args.Length == 0) + { + PrintUsage(); + return 1; + } + + CliArgs parsed = CliArgs.Parse(args, new[] { "threat", "reason" }, Array.Empty()); + if (parsed.Help) + { + PrintUsage(); + return 0; + } + + if (parsed.UnknownFlags.Count > 0) + { + Console.Error.WriteLine("Unknown option: " + parsed.UnknownFlags[0]); + PrintUsage(); + return 1; + } + + string? input = parsed.Positionals.Count > 0 ? parsed.Positionals[0] : null; + string? threatId = parsed.Get("threat"); + string? reason = parsed.Get("reason"); + if (string.IsNullOrEmpty(input) || string.IsNullOrEmpty(threatId)) + { + PrintUsage(); + return 1; + } + + if (string.IsNullOrWhiteSpace(reason)) + { + Console.Error.WriteLine("--reason is required: record why the risk is accepted."); + PrintUsage(); + return 1; + } + + if (!File.Exists(input)) + { + Console.Error.WriteLine("File not found: " + input); + return 1; + } + + (ThreatModel model, IThreatModelFormat? format) = CliModelLoader.Load(input!); + if (format == null || !format.Capabilities.CanWrite) + { + Console.Error.WriteLine("The model's format does not support writing."); + return 1; + } + + if (!ThreatGenerator.Accept(model, threatId!, reason!)) + { + Console.Error.WriteLine("Threat not found: " + threatId + ". Persist threats first with 'tmforge threats --write', then use 'tmforge list threats' to find its id."); + return 1; + } + + AuthoringSupport.Save(model, input!, format); + + if (parsed.Json) + { + CliJson.WriteEnvelope("accept", new + { + threat = threatId, + state = "NotApplicable", + reason, + }); + } + else + { + Console.Error.WriteLine("Accepted " + threatId + " in " + input + ": " + reason); + } + + return 0; + } + + private static void PrintUsage() + { + Console.Error.WriteLine("Accept the risk of a generated threat (marks it not-applicable with a justification)."); + Console.Error.WriteLine("Usage:"); + Console.Error.WriteLine(" tmforge accept --threat --reason [--json] "); + Console.Error.WriteLine(); + Console.Error.WriteLine("--threat accepts the threat's register key, interaction key, or numeric id (from 'tmforge list threats')."); + } + } +} diff --git a/src/ThreatModelForge.Cli/CommandCatalog.cs b/src/ThreatModelForge.Cli/CommandCatalog.cs index f75b6e0..fd862f9 100644 --- a/src/ThreatModelForge.Cli/CommandCatalog.cs +++ b/src/ThreatModelForge.Cli/CommandCatalog.cs @@ -31,6 +31,8 @@ internal static class CommandCatalog new CommandInfo("page", "List, add, rename, reorder, or remove pages (diagrams).", "ls: count, items[]; add: index, name, id; rename: id, name; rm: id, name, remaining; reorder: id, name, index", PageCommand.Run), new CommandInfo("layout", "Auto-lay-out the diagram (layered; no hand-placed coordinates).", "pages, components", LayoutCommand.Run), new CommandInfo("lint", "Validate a threat model against a rule set.", "a SARIF-style model report (runs[].results[]); see docs/cli-reference.md", LintCommand.Run), + new CommandInfo("threats", "Report threats: the persisted, triaged view of the validation findings (--write to persist).", "summary{count,written}, threats[]{id,ruleId,category,title,mitigation,severity,references[],scope,interaction}", ThreatsCommand.Run), + new CommandInfo("accept", "Accept a generated threat's risk (marks it not-applicable with a reason).", "threat, state, reason", AcceptCommand.Run), new CommandInfo("report", "Generate an HTML report from a threat model.", "output, format, bytes", ReportCommand.Run), new CommandInfo("convert", "Convert a threat model between file formats.", "input, output, format", ConvertCommand.Run), new CommandInfo("apply", "Build a model from a declarative JSON manifest (all-or-nothing).", "output, format, dryRun, boundaries, elements, flows", ApplyCommand.Run), diff --git a/src/ThreatModelForge.Cli/ThreatsCommand.cs b/src/ThreatModelForge.Cli/ThreatsCommand.cs new file mode 100644 index 0000000..c575b11 --- /dev/null +++ b/src/ThreatModelForge.Cli/ThreatsCommand.cs @@ -0,0 +1,191 @@ +namespace ThreatModelForge.Cli +{ + using System; + using System.Collections.Generic; + using System.IO; + using System.Linq; + using ThreatModelForge.Analysis; + using ThreatModelForge.Formats; + using ThreatModelForge.Model; + + /// + /// Implements tmforge threats: the persistable, lifecycle-bearing view of the validation + /// findings. It runs the same rules as lint, frames each threat-bearing finding as a STRIDE + /// threat against its element or flow, and — with --write — persists them into the model's + /// register, preserving any prior triage. Detection is entirely the rules; extend coverage by + /// adding a rule to a rule pack (there is no separate threat catalog). + /// + internal static class ThreatsCommand + { + /// + /// Runs the threats command. + /// + /// The command arguments (after the verb). + /// Zero on success; a non-zero value on error. + public static int Run(string[] args) + { + if (args == null || args.Length == 0) + { + PrintUsage(); + return 1; + } + + CliArgs parsed = CliArgs.Parse(args, Array.Empty(), new[] { "write" }); + if (parsed.Help) + { + PrintUsage(); + return 0; + } + + if (parsed.UnknownFlags.Count > 0) + { + Console.Error.WriteLine("Unknown option: " + parsed.UnknownFlags[0]); + PrintUsage(); + return 1; + } + + string? input = parsed.Positionals.Count > 0 ? parsed.Positionals[0] : null; + if (string.IsNullOrEmpty(input)) + { + PrintUsage(); + return 1; + } + + if (!File.Exists(input)) + { + Console.Error.WriteLine("File not found: " + input); + return 1; + } + + (ThreatModel model, IThreatModelFormat? format) = CliModelLoader.Load(input!); + GenerationResult result = ThreatGenerator.Generate(model); + + ApplyResult? written = null; + if (parsed.HasFlag("write")) + { + if (format == null || !format.Capabilities.CanWrite) + { + Console.Error.WriteLine("The model's format does not support writing; omit --write to preview."); + return 1; + } + + written = ThreatGenerator.Apply(model, result); + AuthoringSupport.Save(model, input!, format); + } + + if (parsed.Json) + { + WriteJson(result, written); + } + else + { + WriteHuman(result, written, input!); + } + + return 0; + } + + private static void WriteJson(GenerationResult result, ApplyResult? written) + { + object data = new + { + summary = new + { + count = result.Count, + written = written == null + ? null + : new { added = written.Added, updated = written.Updated, preserved = written.Preserved }, + }, + threats = result.Threats.Select(t => new + { + id = t.Id, + ruleId = t.RuleId, + category = t.Category.ToString(), + title = t.Title, + mitigation = t.Mitigation, + severity = t.Severity, + priority = t.Priority, + references = t.References.Select(r => new { catalog = r.Catalog, id = r.Id, url = r.Url }).ToList(), + scope = t.IsFlowScoped ? "flow" : "element", + source = t.SourceName, + target = t.TargetName, + flow = t.FlowName, + sourceId = t.SourceGuid, + targetId = t.TargetGuid, + flowId = t.FlowGuid, + diagramId = t.DiagramGuid, + interaction = t.InteractionString, + }).ToList(), + }; + CliJson.WriteEnvelope("threats", data); + } + + private static void WriteHuman(GenerationResult result, ApplyResult? written, string input) + { + Console.WriteLine("Threats: " + result.Count + " (from validation findings)"); + if (written != null) + { + Console.WriteLine("Wrote: " + written.Added + " added, " + written.Updated + " updated, " + written.Preserved + " preserved (triaged) in " + input); + } + + if (result.Count == 0) + { + Console.WriteLine(); + Console.WriteLine("No threats detected. Run 'tmforge lint' for the same findings without persisting them."); + return; + } + + Console.WriteLine(); + foreach (GeneratedThreat threat in result.Threats) + { + PrintThreat(threat); + } + } + + private static void PrintThreat(GeneratedThreat threat) + { + string flow = threat.IsFlowScoped && !string.IsNullOrWhiteSpace(threat.FlowName) + ? " [" + threat.FlowName + "]" + : string.Empty; + Console.WriteLine( + " [" + CategoryLetter(threat.Category) + "] " + threat.RuleId + " " + + threat.InteractionString + flow + " — " + threat.Title); + if (!string.IsNullOrWhiteSpace(threat.Mitigation)) + { + Console.WriteLine(" fix: " + threat.Mitigation); + } + + if (threat.References.Count > 0) + { + Console.WriteLine(" refs: " + string.Join(", ", threat.References.Select(r => r.Id))); + } + } + + private static string CategoryLetter(StrideCategory category) + { + return category switch + { + StrideCategory.Spoofing => "S", + StrideCategory.Tampering => "T", + StrideCategory.Repudiation => "R", + StrideCategory.InformationDisclosure => "I", + StrideCategory.DenialOfService => "D", + StrideCategory.ElevationOfPrivilege => "E", + _ => "?", + }; + } + + private static void PrintUsage() + { + Console.Error.WriteLine("Report the model's threats — the persistable, triaged view of the validation findings."); + Console.Error.WriteLine("Usage:"); + Console.Error.WriteLine(" tmforge threats [--write] [--json] "); + Console.Error.WriteLine(); + Console.Error.WriteLine("--write persist the threats into the model's register (preserves prior triage)."); + Console.Error.WriteLine(); + Console.Error.WriteLine("Detection is the rule set (see 'tmforge lint'); each threat carries a STRIDE category,"); + Console.Error.WriteLine("the rule's mitigation, and CWE/CAPEC references. After --write, triage with"); + Console.Error.WriteLine("'tmforge list threats' and 'tmforge accept'."); + } + } +} diff --git a/src/ThreatModelForge.Engine/EngineService.cs b/src/ThreatModelForge.Engine/EngineService.cs index bfe20f1..9c14e39 100644 --- a/src/ThreatModelForge.Engine/EngineService.cs +++ b/src/ThreatModelForge.Engine/EngineService.cs @@ -3,6 +3,7 @@ namespace ThreatModelForge.Engine using System; using System.Collections.Generic; using System.IO; + using System.Linq; using System.Reflection; using System.Text; using System.Text.Json; @@ -178,6 +179,61 @@ public static IReadOnlyList Validate(TmForgeModelDto dto) return findings; } + /// + /// Projects the model's validation findings into STRIDE threats. Detection is entirely the + /// rule set's — this runs the same rules runs and frames the findings + /// from threat-bearing rules as persistable threats. CLI, /v1, and WASM call the same + /// projector, so results are identical by construction. + /// + /// The canonical model. + /// The generated threats. + public static IReadOnlyList GenerateThreats(TmForgeModelDto dto) + { + List result = new List(); + try + { + ThreatModel model = BuildModel(dto, out _); + using (RuleSet ruleSet = LoadRuleSet()) + { + if (dto.Validation != null) + { + ruleSet.Disable(dto.Validation.DisabledPacks, dto.Validation.DisabledRuleIds); + } + + GenerationResult generation = ThreatGenerator.Generate(model, ruleSet); + foreach (GeneratedThreat threat in generation.Threats) + { + result.Add(new ThreatDto + { + Id = threat.Id, + RuleId = threat.RuleId, + Category = threat.Category.ToString(), + Title = threat.Title, + Mitigation = threat.Mitigation, + Severity = threat.Severity, + Priority = threat.Priority, + References = threat.References.Select(r => r.Id).ToList(), + ElementIds = BuildElementIds(threat), + Interaction = threat.InteractionString, + }); + } + } + } +#pragma warning disable CA1031 // Do not catch general exception types + catch (Exception ex) +#pragma warning restore CA1031 // Do not catch general exception types + { + result.Add(new ThreatDto + { + Id = "engine-error", + Severity = "error", + Title = $"Threat generation failed: {ex.Message}", + }); + } + + return result; + } + /// /// Serializes the supplied model to lossless .tm7 bytes via the real engine. /// @@ -325,6 +381,18 @@ public static MergeResultDto Merge(TmForgeModelDto? baseModel, TmForgeModelDto o return new MergeResultDto { Merged = ToDto(result.Merged), Conflicts = conflicts }; } + private static IReadOnlyList BuildElementIds(GeneratedThreat threat) + { + List ids = new List { threat.SourceGuid.ToString() }; + if (threat.IsFlowScoped) + { + ids.Add(threat.TargetGuid.ToString()); + ids.Add(threat.FlowGuid.ToString()); + } + + return ids; + } + private static ThreatModel BuildModel(TmForgeModelDto dto, out Dictionary> nameToIds) { nameToIds = new Dictionary>(StringComparer.Ordinal); diff --git a/src/ThreatModelForge.Engine/ThreatDto.cs b/src/ThreatModelForge.Engine/ThreatDto.cs new file mode 100644 index 0000000..f15d106 --- /dev/null +++ b/src/ThreatModelForge.Engine/ThreatDto.cs @@ -0,0 +1,41 @@ +namespace ThreatModelForge.Engine +{ + using System; + using System.Collections.Generic; + + /// + /// A single generated STRIDE threat returned to the client, projected from a validation finding. + /// + public sealed class ThreatDto + { + /// Gets the deterministic threat identifier (register key). + public string Id { get; init; } = string.Empty; + + /// Gets the identifier of the rule that detected the threat (for example TM1023). + public string RuleId { get; init; } = string.Empty; + + /// Gets the STRIDE category. + public string Category { get; init; } = string.Empty; + + /// Gets the threat title (the finding text). + public string Title { get; init; } = string.Empty; + + /// Gets the suggested mitigation (the rule's help text). + public string? Mitigation { get; init; } + + /// Gets the rule severity (error, warning, or info). + public string Severity { get; init; } = "warning"; + + /// Gets the coarse priority hint (High / Medium / Low). + public string? Priority { get; init; } + + /// Gets the external catalog reference identifiers (CAPEC / CWE / ATT&CK). + public IReadOnlyList References { get; init; } = Array.Empty(); + + /// Gets the element identifiers (source, target, flow) this threat refers to, for highlighting. + public IReadOnlyList ElementIds { get; init; } = Array.Empty(); + + /// Gets the human-readable scope string (source -> target for a flow, else the element name). + public string Interaction { get; init; } = string.Empty; + } +} diff --git a/test/ThreatModelForge.Analysis.Rules.Tests/ConformanceTests.cs b/test/ThreatModelForge.Analysis.Rules.Tests/ConformanceTests.cs index f826b6f..790909c 100644 --- a/test/ThreatModelForge.Analysis.Rules.Tests/ConformanceTests.cs +++ b/test/ThreatModelForge.Analysis.Rules.Tests/ConformanceTests.cs @@ -49,6 +49,35 @@ public void BasicPropertiesTest() Assert.IsTrue(errors.Length == 0, errors.ToString()); } + /// + /// Tests that each rule's threat identity is complete and self-contained: a rule that declares + /// a STRIDE category (a threat-bearing rule) also carries at least one external reference, and + /// references are never declared without a category. This is the anti-drift guard — because a + /// rule owns its own STRIDE/references, adding a rule cannot silently forget to make it a + /// threat, and there is no separate map to fall out of sync. + /// + [TestMethod] + public void ThreatMetadataIsConsistentTest() + { + StringBuilder errors = new (); + foreach (Rule rule in GetInstancesOfEachRule()) + { + bool hasStride = rule.Stride.HasValue; + bool hasReferences = rule.ThreatReferences.Count > 0; + if (hasStride && !hasReferences) + { + errors.AppendLine(CultureInfo.InvariantCulture, $"Rule {rule.GetType()} declares Stride {rule.Stride} but has no ThreatReferences."); + } + + if (!hasStride && hasReferences) + { + errors.AppendLine(CultureInfo.InvariantCulture, $"Rule {rule.GetType()} has ThreatReferences but no Stride category."); + } + } + + Assert.IsTrue(errors.Length == 0, errors.ToString()); + } + /// /// Tests each rule has its own unique Help URI. /// diff --git a/test/ThreatModelForge.Analysis.Rules.Tests/ThreatGeneratorTest.cs b/test/ThreatModelForge.Analysis.Rules.Tests/ThreatGeneratorTest.cs new file mode 100644 index 0000000..b0d5a58 --- /dev/null +++ b/test/ThreatModelForge.Analysis.Rules.Tests/ThreatGeneratorTest.cs @@ -0,0 +1,165 @@ +namespace ThreatModelForge.Analysis.Rules.Tests +{ + using System; + using System.Linq; + using Microsoft.VisualStudio.TestTools.UnitTesting; + using ThreatModelForge.KnowledgeBase; + using ThreatModelForge.Model; + using ThreatModelForge.Model.Abstracts; + + /// + /// Unit tests for : it projects the real rule findings into threats + /// (no independent detection), persists them idempotently, preserves triage, and records + /// acceptance. + /// + [TestClass] + public class ThreatGeneratorTest + { + private const string ProcessTypeId = "GE.P"; + private const string ExternalTypeId = "GE.EI"; + + /// An unauthenticated external interactor produces a spoofing threat from rule TM1023. + [TestMethod] + public void GenerateProducesSpoofingThreatFromRule() + { + ThreatModel model = UnauthenticatedExternalModel(); + + GenerationResult result = ThreatGenerator.Generate(model); + + GeneratedThreat? spoof = result.Threats.FirstOrDefault(t => t.RuleId == "TM1023"); + Assert.IsNotNull(spoof); + Assert.AreEqual(StrideCategory.Spoofing, spoof!.Category); + Assert.IsFalse(string.IsNullOrEmpty(spoof.Mitigation)); + Assert.IsTrue(spoof.References.Any(r => r.Id == "CWE-287")); + } + + /// Every generated threat comes from a threat-bearing rule; hygiene findings are excluded. + [TestMethod] + public void GeneratedThreatsAreOnlyThreatBearingRules() + { + ThreatModel model = UnauthenticatedExternalModel(); + + GenerationResult result = ThreatGenerator.Generate(model); + + Assert.IsTrue(result.Count > 0); + foreach (GeneratedThreat threat in result.Threats) + { + // Every generated threat comes from a rule that declares its STRIDE identity, so it + // carries the rule's references. Hygiene rules (no Stride) never reach here. + Assert.IsTrue(threat.References.Count > 0, $"Threat {threat.RuleId} should carry its rule's references."); + } + + // TM1005 (minimum component count) is a hygiene rule and must never surface as a threat. + Assert.IsFalse(result.Threats.Any(t => t.RuleId == "TM1005")); + Assert.IsTrue(result.Threats.Any(t => t.RuleId == "TM1023")); + } + + /// persists the threats into the register. + [TestMethod] + public void ApplyPersistsThreats() + { + ThreatModel model = UnauthenticatedExternalModel(); + GenerationResult result = ThreatGenerator.Generate(model); + + ApplyResult applied = ThreatGenerator.Apply(model, result); + + Assert.AreEqual(result.Count, applied.Added); + Assert.AreEqual(result.Count, model.AllThreatsDictionary.Count); + Assert.IsTrue(model.ThreatGenerationEnabled ?? false); + } + + /// Re-running generation and apply is idempotent: nothing is added the second time. + [TestMethod] + public void ApplyIsIdempotent() + { + ThreatModel model = UnauthenticatedExternalModel(); + ThreatGenerator.Apply(model, ThreatGenerator.Generate(model)); + int count = model.AllThreatsDictionary.Count; + + ApplyResult second = ThreatGenerator.Apply(model, ThreatGenerator.Generate(model)); + + Assert.AreEqual(0, second.Added); + Assert.AreEqual(count, model.AllThreatsDictionary.Count); + } + + /// Re-generation preserves a human-triaged threat rather than overwriting it. + [TestMethod] + public void ApplyPreservesTriagedThreats() + { + ThreatModel model = UnauthenticatedExternalModel(); + ThreatGenerator.Apply(model, ThreatGenerator.Generate(model)); + string key = model.AllThreatsDictionary.Keys.First(); + model.AllThreatsDictionary[key].State = ThreatState.Mitigated; + + ApplyResult second = ThreatGenerator.Apply(model, ThreatGenerator.Generate(model)); + + Assert.IsTrue(second.Preserved >= 1); + Assert.AreEqual(ThreatState.Mitigated, model.AllThreatsDictionary[key].State); + } + + /// Accepting a threat marks it not-applicable and records the justification. + [TestMethod] + public void AcceptMarksThreatNotApplicableWithJustification() + { + ThreatModel model = UnauthenticatedExternalModel(); + ThreatGenerator.Apply(model, ThreatGenerator.Generate(model)); + string key = model.AllThreatsDictionary.Keys.First(); + + bool accepted = ThreatGenerator.Accept(model, key, "Handled upstream"); + + Assert.IsTrue(accepted); + Assert.AreEqual(ThreatState.NotApplicable, model.AllThreatsDictionary[key].State); + Assert.AreEqual("Handled upstream", model.AllThreatsDictionary[key].StateInformation); + } + + /// Accepting an unknown identifier reports failure. + [TestMethod] + public void AcceptUnknownThreatReturnsFalse() + { + ThreatModel model = UnauthenticatedExternalModel(); + ThreatGenerator.Apply(model, ThreatGenerator.Generate(model)); + + Assert.IsFalse(ThreatGenerator.Accept(model, "does-not-exist", "reason")); + } + + private static ThreatModel UnauthenticatedExternalModel() + { + StencilRectangle external = External("Client"); + StencilEllipse process = Process("Gateway"); + Connector flow = Flow(external, process, "request"); + DrawingSurfaceModel diagram = new DrawingSurfaceModel { Header = "DFD" }; + diagram.Borders.Add(external.Guid, external); + diagram.Borders.Add(process.Guid, process); + diagram.Lines.Add(flow.Guid, flow); + ThreatModel model = new ThreatModel(); + model.DrawingSurfaceList.Add(diagram); + return model; + } + + private static StencilRectangle External(string name) + { + StencilRectangle entity = new StencilRectangle { Guid = Guid.NewGuid(), GenericTypeId = ExternalTypeId }; + SetName(entity, name); + return entity; + } + + private static StencilEllipse Process(string name) + { + StencilEllipse entity = new StencilEllipse { Guid = Guid.NewGuid(), GenericTypeId = ProcessTypeId }; + SetName(entity, name); + return entity; + } + + private static Connector Flow(Entity source, Entity target, string name) + { + Connector connector = new Connector { Guid = Guid.NewGuid(), SourceGuid = source.Guid, TargetGuid = target.Guid }; + SetName(connector, name); + return connector; + } + + private static void SetName(Entity entity, string name) + { + entity.Properties.Add(new StringDisplayAttribute { DisplayName = "Name", Value = name }); + } + } +} diff --git a/test/ThreatModelForge.Api.Tests/EngineGenerateThreatsTest.cs b/test/ThreatModelForge.Api.Tests/EngineGenerateThreatsTest.cs new file mode 100644 index 0000000..0af2edf --- /dev/null +++ b/test/ThreatModelForge.Api.Tests/EngineGenerateThreatsTest.cs @@ -0,0 +1,56 @@ +namespace ThreatModelForge.Api.Tests +{ + using System.Collections.Generic; + using System.Linq; + using Microsoft.VisualStudio.TestTools.UnitTesting; + using ThreatModelForge.Engine; + + /// + /// Unit tests for , confirming the shared engine seam + /// projects the same rule findings the CLI does (parity by construction) and honors the per-model + /// rule selection. + /// + [TestClass] + public class EngineGenerateThreatsTest + { + /// An unauthenticated external interactor yields a spoofing threat from rule TM1023. + [TestMethod] + public void GenerateThreats_ProducesSpoofingFromRule() + { + IReadOnlyList threats = EngineService.GenerateThreats(BuildModel(null)); + + ThreatDto? spoof = threats.FirstOrDefault(t => t.RuleId == "TM1023"); + Assert.IsNotNull(spoof); + Assert.AreEqual("Spoofing", spoof!.Category); + Assert.IsTrue(spoof.References.Contains("CWE-287")); + } + + /// Disabling the rule's pack removes its threat — detection and generation share the rule set. + [TestMethod] + public void GenerateThreats_HonorsDisabledPack() + { + TmForgeValidationDto validation = new TmForgeValidationDto + { + DisabledPacks = new[] { "identity-access" }, + }; + + IReadOnlyList threats = EngineService.GenerateThreats(BuildModel(validation)); + + Assert.IsFalse(threats.Any(t => t.RuleId == "TM1023")); + } + + private static TmForgeModelDto BuildModel(TmForgeValidationDto? validation) + { + TmForgeElementDto external = new TmForgeElementDto { Id = "e1", Kind = "external", Name = "Client", X = 50, Y = 50 }; + TmForgeElementDto process = new TmForgeElementDto { Id = "p1", Kind = "process", Name = "Gateway", X = 220, Y = 50 }; + return new TmForgeModelDto + { + Schema = "tmforge-json", + Version = "0.1", + Elements = new[] { external, process }, + Flows = new[] { new TmForgeFlowDto { Id = "f1", Source = "e1", Target = "p1", Name = "request" } }, + Validation = validation, + }; + } + } +} diff --git a/test/ThreatModelForge.Cli.Tests/ThreatsCommandTest.cs b/test/ThreatModelForge.Cli.Tests/ThreatsCommandTest.cs new file mode 100644 index 0000000..f9e0a4f --- /dev/null +++ b/test/ThreatModelForge.Cli.Tests/ThreatsCommandTest.cs @@ -0,0 +1,172 @@ +namespace ThreatModelForge.Cli.Tests +{ + using System; + using System.IO; + using System.Linq; + using System.Text.Json; + using Microsoft.VisualStudio.TestTools.UnitTesting; + using ThreatModelForge.KnowledgeBase; + using ThreatModelForge.Model; + + /// + /// Integration tests for the tmforge threats and tmforge accept commands: threats are + /// the persisted, triaged view of the validation findings — persistence, idempotency, and + /// acceptance through the real CLI entry points. + /// + [TestClass] + public class ThreatsCommandTest + { + /// Gets or sets the working directory created for each test. + private string WorkingDirectory { get; set; } = string.Empty; + + /// Gets the path of the model created by . + private string ModelPath => Path.Combine(this.WorkingDirectory, "model.tm7"); + + /// Creates an isolated working directory for the test. + [TestInitialize] + public void Initialize() + { + this.WorkingDirectory = Path.Combine(Path.GetTempPath(), "tmforge-threats-" + Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(this.WorkingDirectory); + } + + /// Removes the working directory after the test. + [TestCleanup] + public void Cleanup() + { + if (Directory.Exists(this.WorkingDirectory)) + { + Directory.Delete(this.WorkingDirectory, recursive: true); + } + } + + /// Threats are reported from the validation findings (an unauthenticated external → TM1023). + [TestMethod] + public void ThreatsReportsThreatsFromFindings() + { + string path = this.NewModelWithFlow(); + + (int exit, string stdout) = Capture(() => ThreatsCommand.Run(new[] { path, "--json" })); + + Assert.AreEqual(0, exit); + JsonElement data = JsonDocument.Parse(stdout).RootElement.GetProperty("data"); + Assert.IsTrue(data.GetProperty("summary").GetProperty("count").GetInt32() > 0); + Assert.IsTrue(HasRule(data, "TM1023")); + } + + /// Writing persists the threats into the model's register. + [TestMethod] + public void ThreatsWritePersistsRegister() + { + string path = this.NewModelWithFlow(); + + (int exit, string stdout) = Capture(() => ThreatsCommand.Run(new[] { path, "--write", "--json" })); + + Assert.AreEqual(0, exit); + JsonElement data = JsonDocument.Parse(stdout).RootElement.GetProperty("data"); + int added = data.GetProperty("summary").GetProperty("written").GetProperty("added").GetInt32(); + Assert.IsTrue(added > 0); + (ThreatModel model, _) = CliModelLoader.Load(path); + Assert.AreEqual(added, model.AllThreatsDictionary.Count); + } + + /// Writing twice is idempotent — nothing is added the second time. + [TestMethod] + public void ThreatsWriteIsIdempotent() + { + string path = this.NewModelWithFlow(); + Capture(() => ThreatsCommand.Run(new[] { path, "--write" })); + + (int exit, string stdout) = Capture(() => ThreatsCommand.Run(new[] { path, "--write", "--json" })); + + Assert.AreEqual(0, exit); + JsonElement data = JsonDocument.Parse(stdout).RootElement.GetProperty("data"); + Assert.AreEqual(0, data.GetProperty("summary").GetProperty("written").GetProperty("added").GetInt32()); + } + + /// Accepting a persisted threat marks it not-applicable with the justification. + [TestMethod] + public void AcceptMarksThreatNotApplicable() + { + string path = this.NewModelWithFlow(); + (_, string writeOut) = Capture(() => ThreatsCommand.Run(new[] { path, "--write", "--json" })); + JsonElement data = JsonDocument.Parse(writeOut).RootElement.GetProperty("data"); + string id = data.GetProperty("threats").EnumerateArray().First().GetProperty("id").GetString() ?? string.Empty; + + (int exit, _) = Capture(() => AcceptCommand.Run(new[] { path, "--threat", id, "--reason", "Accepted for test" })); + + Assert.AreEqual(0, exit); + (ThreatModel model, _) = CliModelLoader.Load(path); + Assert.IsTrue(model.AllThreatsDictionary.TryGetValue(id, out Threat? threat)); + Assert.AreEqual(ThreatState.NotApplicable, threat!.State); + Assert.AreEqual("Accepted for test", threat.StateInformation); + } + + /// Accepting without a reason is reported as an error. + [TestMethod] + public void AcceptWithoutReasonReturnsError() + { + string path = this.NewModelWithFlow(); + + (int exit, _) = Capture(() => AcceptCommand.Run(new[] { path, "--threat", "anything" })); + + Assert.AreEqual(1, exit); + } + + private static (int Exit, string Stdout) Capture(Func run) + { + StringWriter outWriter = new StringWriter(); + StringWriter errorWriter = new StringWriter(); + TextWriter originalOut = Console.Out; + TextWriter originalError = Console.Error; + Console.SetOut(outWriter); + Console.SetError(errorWriter); + try + { + int exit = run(); + return (exit, outWriter.ToString()); + } + finally + { + Console.SetOut(originalOut); + Console.SetError(originalError); + } + } + + private static bool HasRule(JsonElement data, string ruleId) + { + foreach (JsonElement threat in data.GetProperty("threats").EnumerateArray()) + { + if (threat.GetProperty("ruleId").GetString() == ruleId) + { + return true; + } + } + + return false; + } + + private string NewModelWithFlow() + { + Capture(() => NewCommand.Run(new[] { this.ModelPath, "--name", "Threats Test" })); + string external = this.AddElement("external", "Client"); + string process = this.AddElement("process", "Gateway"); + this.Connect(external, process); + return this.ModelPath; + } + + private string AddElement(string kind, string name) + { + (int exit, string stdout) = Capture(() => AddCommand.Run(new[] { kind, this.ModelPath, "--name", name, "--json" })); + Assert.AreEqual(0, exit); + return JsonDocument.Parse(stdout).RootElement.GetProperty("data").GetProperty("id").GetGuid().ToString(); + } + + private string Connect(string source, string target) + { + (int exit, string stdout) = Capture(() => ConnectCommand.Run(new[] { this.ModelPath, "--source", source, "--target", target, "--json" })); + Assert.AreEqual(0, exit); + return JsonDocument.Parse(stdout).RootElement.GetProperty("data").GetProperty("id").GetGuid().ToString(); + } + } +} From ec784020696e68b6fccfcead1a997aa69680d5f0 Mon Sep 17 00:00:00 2001 From: Hacks4Snacks Date: Tue, 7 Jul 2026 23:28:33 -0500 Subject: [PATCH 2/8] updated ruleset --- .../DataStoreMissingBackupRule.cs | 98 +++++++++ ...toreMissingBackupRuleResources.Designer.cs | 90 ++++++++ .../DataStoreMissingBackupRuleResources.resx | 72 +++++++ .../RuleIDs.cs | 15 ++ .../RulePackCatalog.cs | 4 + .../SensitiveDataToExternalRule.cs | 120 +++++++++++ ...iveDataToExternalRuleResources.Designer.cs | 90 ++++++++ .../SensitiveDataToExternalRuleResources.resx | 72 +++++++ .../UnauditedBoundaryProcessRule.cs | 109 ++++++++++ ...edBoundaryProcessRuleResources.Designer.cs | 90 ++++++++ ...UnauditedBoundaryProcessRuleResources.resx | 72 +++++++ src/ThreatModelForge.Analysis/README.md | 2 + .../DataStoreMissingBackupRuleTests.cs | 112 ++++++++++ .../SensitiveDataToExternalRuleTests.cs | 139 ++++++++++++ .../UnauditedBoundaryProcessRuleTests.cs | 197 ++++++++++++++++++ .../RuleCatalogTest.cs | 3 +- 16 files changed, 1284 insertions(+), 1 deletion(-) create mode 100644 src/ThreatModelForge.Analysis.Rules/DataStoreMissingBackupRule.cs create mode 100644 src/ThreatModelForge.Analysis.Rules/DataStoreMissingBackupRuleResources.Designer.cs create mode 100644 src/ThreatModelForge.Analysis.Rules/DataStoreMissingBackupRuleResources.resx create mode 100644 src/ThreatModelForge.Analysis.Rules/SensitiveDataToExternalRule.cs create mode 100644 src/ThreatModelForge.Analysis.Rules/SensitiveDataToExternalRuleResources.Designer.cs create mode 100644 src/ThreatModelForge.Analysis.Rules/SensitiveDataToExternalRuleResources.resx create mode 100644 src/ThreatModelForge.Analysis.Rules/UnauditedBoundaryProcessRule.cs create mode 100644 src/ThreatModelForge.Analysis.Rules/UnauditedBoundaryProcessRuleResources.Designer.cs create mode 100644 src/ThreatModelForge.Analysis.Rules/UnauditedBoundaryProcessRuleResources.resx create mode 100644 test/ThreatModelForge.Analysis.Rules.Tests/DataStoreMissingBackupRuleTests.cs create mode 100644 test/ThreatModelForge.Analysis.Rules.Tests/SensitiveDataToExternalRuleTests.cs create mode 100644 test/ThreatModelForge.Analysis.Rules.Tests/UnauditedBoundaryProcessRuleTests.cs diff --git a/src/ThreatModelForge.Analysis.Rules/DataStoreMissingBackupRule.cs b/src/ThreatModelForge.Analysis.Rules/DataStoreMissingBackupRule.cs new file mode 100644 index 0000000..3a2f0a8 --- /dev/null +++ b/src/ThreatModelForge.Analysis.Rules/DataStoreMissingBackupRule.cs @@ -0,0 +1,98 @@ +namespace ThreatModelForge.Analysis.Rules +{ + using System; + using ThreatModelForge.Model; + using ThreatModelForge.Model.Abstracts; + + /// + /// Rule that flags a data store holding important data (credentials or audit/log data) that declares + /// no backup, so destruction or corruption of the store's data is unrecoverable. + /// + /// + /// A data store flagged as holding credentials (StoresCredentials = Yes) or log/audit data + /// (StoresLogData = Yes) whose Backup property is No or unset has no recovery path: + /// accidental deletion, corruption, or a destructive attack (for example ransomware) permanently loses + /// the secrets an application depends on or the audit trail that proves what happened — a loss of + /// availability, and for audit data a loss of the evidence needed for non-repudiation. + /// + public class DataStoreMissingBackupRule : Rule + { + /// + /// Custom attribute that records whether a store is backed up. + /// + public const string BackupCustomAttributeName = "Backup"; + + /// + /// Initializes a new instance of the class. + /// + public DataStoreMissingBackupRule() + : base(RuleIDs.DataStoreMissingBackupRule, MessageSeverity.Warning, RulePackCatalog.Availability) + { + this.FullDescription = DataStoreMissingBackupRuleResources.FullDescription; + this.HelpText = DataStoreMissingBackupRuleResources.HelpText; + this.HelpUri = RuleDocumentation.HelpUriFor(this.ID); + } + + /// + public override IReadOnlyList PropertyBindings => new[] + { + new PropertyBinding("datastore", BackupCustomAttributeName, "No"), + new PropertyBinding("datastore", "StoresCredentials"), + new PropertyBinding("datastore", "StoresLogData"), + }; + + /// + public override StrideCategory? Stride => StrideCategory.DenialOfService; + + /// + public override IReadOnlyList ThreatReferences => new[] + { + ThreatReference.Attack("T1485"), + ThreatReference.Attack("T1490"), + }; + + /// + public override void Evaluate(RuleEvaluationContext context) + { + _ = context ?? throw new ArgumentNullException(nameof(context)); + + foreach (DrawingSurfaceModel diagram in context.Model.DrawingSurfaceList) + { + foreach (Entity component in diagram.Components()) + { + if (!component.IsStorageComponent()) + { + continue; + } + + if (!HoldsImportantData(component) || IsBackedUp(component)) + { + continue; + } + + string text = string.Format( + System.Globalization.CultureInfo.CurrentCulture, + DataStoreMissingBackupRuleResources.MessageText, + GetEntityDisplayText(component)); + context.Writer.Write(this.CreateMessage(component, diagram, text)); + } + } + } + + private static bool HoldsImportantData(Entity component) + { + return IsYes(component, "StoresCredentials") || IsYes(component, "StoresLogData"); + } + + private static bool IsBackedUp(Entity component) + { + return IsYes(component, BackupCustomAttributeName); + } + + private static bool IsYes(Entity component, string propertyName) + { + return component.TryGetCustomPropertyValue(propertyName, out string? value) && + string.Equals(value, "Yes", StringComparison.OrdinalIgnoreCase); + } + } +} diff --git a/src/ThreatModelForge.Analysis.Rules/DataStoreMissingBackupRuleResources.Designer.cs b/src/ThreatModelForge.Analysis.Rules/DataStoreMissingBackupRuleResources.Designer.cs new file mode 100644 index 0000000..0ba9fef --- /dev/null +++ b/src/ThreatModelForge.Analysis.Rules/DataStoreMissingBackupRuleResources.Designer.cs @@ -0,0 +1,90 @@ +//------------------------------------------------------------------------------ +// +// This code was generated by a tool. +// Runtime Version:4.0.30319.42000 +// +// Changes to this file may cause incorrect behavior and will be lost if +// the code is regenerated. +// +//------------------------------------------------------------------------------ + +namespace ThreatModelForge.Analysis.Rules { + using System; + + + /// + /// A strongly-typed resource class, for looking up localized strings, etc. + /// + // This class was auto-generated by the StronglyTypedResourceBuilder + // class via a tool like ResGen or Visual Studio. + // To add or remove a member, edit your .ResX file then rerun ResGen + // with the /str option, or rebuild your VS project. + [global::System.CodeDom.Compiler.GeneratedCodeAttribute("System.Resources.Tools.StronglyTypedResourceBuilder", "17.0.0.0")] + [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] + [global::System.Runtime.CompilerServices.CompilerGeneratedAttribute()] + internal class DataStoreMissingBackupRuleResources { + + private static global::System.Resources.ResourceManager resourceMan; + + private static global::System.Globalization.CultureInfo resourceCulture; + + [global::System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("Microsoft.Performance", "CA1811:AvoidUncalledPrivateCode")] + internal DataStoreMissingBackupRuleResources() { + } + + /// + /// Returns the cached ResourceManager instance used by this class. + /// + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Advanced)] + internal static global::System.Resources.ResourceManager ResourceManager { + get { + if (object.ReferenceEquals(resourceMan, null)) { + global::System.Resources.ResourceManager temp = new global::System.Resources.ResourceManager("ThreatModelForge.Analysis.Rules.DataStoreMissingBackupRuleResources", typeof(DataStoreMissingBackupRuleResources).Assembly); + resourceMan = temp; + } + return resourceMan; + } + } + + /// + /// Overrides the current thread's CurrentUICulture property for all + /// resource lookups using this strongly typed resource class. + /// + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Advanced)] + internal static global::System.Globalization.CultureInfo Culture { + get { + return resourceCulture; + } + set { + resourceCulture = value; + } + } + + /// + /// Looks up a localized string similar to the full description of the data-store-missing-backup rule. + /// + internal static string FullDescription { + get { + return ResourceManager.GetString("FullDescription", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to the help text of the data-store-missing-backup rule. + /// + internal static string HelpText { + get { + return ResourceManager.GetString("HelpText", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to The {0} holds important data but declares no backup. + /// + internal static string MessageText { + get { + return ResourceManager.GetString("MessageText", resourceCulture); + } + } + } +} diff --git a/src/ThreatModelForge.Analysis.Rules/DataStoreMissingBackupRuleResources.resx b/src/ThreatModelForge.Analysis.Rules/DataStoreMissingBackupRuleResources.resx new file mode 100644 index 0000000..9bc9784 --- /dev/null +++ b/src/ThreatModelForge.Analysis.Rules/DataStoreMissingBackupRuleResources.resx @@ -0,0 +1,72 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + text/microsoft-resx + + + 2.0 + + + System.Resources.ResXResourceReader, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + + System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + + A data store that holds important data but has no backup cannot be recovered if its contents are deleted, corrupted, or destroyed. Loss of a credential store strands the application that depends on those secrets; loss of an audit-log store destroys the evidence needed to prove what happened. Either outcome is a loss of availability, and destroying an audit trail also enables repudiation. + +This rule fires when a storage component flagged as holding credentials (StoresCredentials = Yes) or log/audit data (StoresLogData = Yes) has its Backup property set to No or left unset. Stores that are not flagged as holding important data are not reported. + + + Set Backup = Yes on the store once a tested backup and restore path exists, so the credentials or audit trail it holds can be recovered after accidental deletion, corruption, or a destructive attack. Keep at least one backup copy isolated from the primary store so a single compromise cannot destroy both. + + + The {0} holds important data but declares no backup; set Backup = Yes with a tested restore path so its contents can be recovered after loss or a destructive attack. + + diff --git a/src/ThreatModelForge.Analysis.Rules/RuleIDs.cs b/src/ThreatModelForge.Analysis.Rules/RuleIDs.cs index 08221d7..f0b1368 100644 --- a/src/ThreatModelForge.Analysis.Rules/RuleIDs.cs +++ b/src/ThreatModelForge.Analysis.Rules/RuleIDs.cs @@ -144,5 +144,20 @@ internal class RuleIDs /// Rule that flags a flow that caches a credential read from a credential store. /// public const int CachedCredentialReadRule = 1027; + + /// + /// Rule that flags a data store holding important data (credentials or audit logs) with no backup. + /// + public const int DataStoreMissingBackupRule = 1028; + + /// + /// Rule that flags a process receiving cross-boundary input with no reachable audit-log store. + /// + public const int UnauditedBoundaryProcessRule = 1029; + + /// + /// Rule that flags a flow carrying sensitive data whose destination is an external interactor. + /// + public const int SensitiveDataToExternalRule = 1030; } } diff --git a/src/ThreatModelForge.Analysis.Rules/RulePackCatalog.cs b/src/ThreatModelForge.Analysis.Rules/RulePackCatalog.cs index 3e763a7..64c0dfe 100644 --- a/src/ThreatModelForge.Analysis.Rules/RulePackCatalog.cs +++ b/src/ThreatModelForge.Analysis.Rules/RulePackCatalog.cs @@ -29,6 +29,9 @@ public static class RulePackCatalog /// Rules that check authentication, least privilege, and access to components. public const string IdentityAccess = "identity-access"; + /// Rules that check availability and recoverability: backups, redundancy, and audit-trail durability. + public const string Availability = "availability"; + private static readonly IReadOnlyList> OrderedPacks = new List> { new KeyValuePair(CoreHygiene, "Core Hygiene"), @@ -37,6 +40,7 @@ public static class RulePackCatalog new KeyValuePair(DataProtection, "Data Protection"), new KeyValuePair(TransportSecurity, "Transport Security"), new KeyValuePair(IdentityAccess, "Identity & Access"), + new KeyValuePair(Availability, "Availability"), }; /// diff --git a/src/ThreatModelForge.Analysis.Rules/SensitiveDataToExternalRule.cs b/src/ThreatModelForge.Analysis.Rules/SensitiveDataToExternalRule.cs new file mode 100644 index 0000000..9e01b5e --- /dev/null +++ b/src/ThreatModelForge.Analysis.Rules/SensitiveDataToExternalRule.cs @@ -0,0 +1,120 @@ +namespace ThreatModelForge.Analysis.Rules +{ + using System; + using System.Collections.Generic; + using System.Linq; + using ThreatModelForge.Model; + using ThreatModelForge.Model.Abstracts; + + /// + /// Rule that flags a flow carrying sensitive data whose destination is an external interactor, so the + /// data leaves the system's trust into a party the model does not control. + /// + /// + /// A flow whose data classification (DataType) is sensitive — end-user identifiable or + /// pseudonymous information, customer content, account data, or access-control data — that terminates + /// at an external interactor sends that data outside the system boundary. Unless the disclosure is + /// intended and the external party is authorized to receive it, this is an information-disclosure path + /// that should be justified or removed. + /// + public class SensitiveDataToExternalRule : Rule + { + /// + /// Custom attribute that records the data classification of a flow. + /// + public const string DataTypeCustomAttributeName = "DataType"; + + /// + /// The data classifications treated as sensitive for the purpose of this rule. + /// + private static readonly HashSet SensitiveClassifications = new HashSet(StringComparer.OrdinalIgnoreCase) + { + "EUII", + "EUPI", + "Customer Content", + "Account Data", + "Access Control Data", + }; + + /// + /// Initializes a new instance of the class. + /// + public SensitiveDataToExternalRule() + : base(RuleIDs.SensitiveDataToExternalRule, MessageSeverity.Warning, RulePackCatalog.DataProtection) + { + this.FullDescription = SensitiveDataToExternalRuleResources.FullDescription; + this.HelpText = SensitiveDataToExternalRuleResources.HelpText; + this.HelpUri = RuleDocumentation.HelpUriFor(this.ID); + } + + /// + public override IReadOnlyList PropertyBindings => new[] + { + new PropertyBinding("flow", DataTypeCustomAttributeName, "EUII", "EUPI", "Customer Content", "Account Data", "Access Control Data"), + }; + + /// + public override StrideCategory? Stride => StrideCategory.InformationDisclosure; + + /// + public override IReadOnlyList ThreatReferences => new[] + { + ThreatReference.Cwe(200), + }; + + /// + public override void Evaluate(RuleEvaluationContext context) + { + _ = context ?? throw new ArgumentNullException(nameof(context)); + + foreach (DrawingSurfaceModel diagram in context.Model.DrawingSurfaceList) + { + foreach (Connector connector in diagram.Lines.Values.OfType()) + { + if (!TryGetSensitiveClassification(connector, out string classification)) + { + continue; + } + + if (!TargetIsExternalInteractor(diagram, connector)) + { + continue; + } + + string text = string.Format( + System.Globalization.CultureInfo.CurrentCulture, + SensitiveDataToExternalRuleResources.MessageText, + GetEntityDisplayText(connector), + classification); + context.Writer.Write(this.CreateMessage(connector, diagram, text)); + } + } + } + + private static bool TryGetSensitiveClassification(Connector connector, out string classification) + { + classification = string.Empty; + if (!connector.TryGetCustomPropertyValue(DataTypeCustomAttributeName, out string? value) || + string.IsNullOrWhiteSpace(value)) + { + return false; + } + + string trimmed = value!.Trim(); + if (!SensitiveClassifications.Contains(trimmed)) + { + return false; + } + + classification = trimmed; + return true; + } + + private static bool TargetIsExternalInteractor(DrawingSurfaceModel diagram, Connector connector) + { + return diagram.Borders.TryGetValue(connector.TargetGuid, out object? value) && + value is Entity target && + target.IsExternalInteractor(); + } + } +} diff --git a/src/ThreatModelForge.Analysis.Rules/SensitiveDataToExternalRuleResources.Designer.cs b/src/ThreatModelForge.Analysis.Rules/SensitiveDataToExternalRuleResources.Designer.cs new file mode 100644 index 0000000..fa1cc4d --- /dev/null +++ b/src/ThreatModelForge.Analysis.Rules/SensitiveDataToExternalRuleResources.Designer.cs @@ -0,0 +1,90 @@ +//------------------------------------------------------------------------------ +// +// This code was generated by a tool. +// Runtime Version:4.0.30319.42000 +// +// Changes to this file may cause incorrect behavior and will be lost if +// the code is regenerated. +// +//------------------------------------------------------------------------------ + +namespace ThreatModelForge.Analysis.Rules { + using System; + + + /// + /// A strongly-typed resource class, for looking up localized strings, etc. + /// + // This class was auto-generated by the StronglyTypedResourceBuilder + // class via a tool like ResGen or Visual Studio. + // To add or remove a member, edit your .ResX file then rerun ResGen + // with the /str option, or rebuild your VS project. + [global::System.CodeDom.Compiler.GeneratedCodeAttribute("System.Resources.Tools.StronglyTypedResourceBuilder", "17.0.0.0")] + [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] + [global::System.Runtime.CompilerServices.CompilerGeneratedAttribute()] + internal class SensitiveDataToExternalRuleResources { + + private static global::System.Resources.ResourceManager resourceMan; + + private static global::System.Globalization.CultureInfo resourceCulture; + + [global::System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("Microsoft.Performance", "CA1811:AvoidUncalledPrivateCode")] + internal SensitiveDataToExternalRuleResources() { + } + + /// + /// Returns the cached ResourceManager instance used by this class. + /// + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Advanced)] + internal static global::System.Resources.ResourceManager ResourceManager { + get { + if (object.ReferenceEquals(resourceMan, null)) { + global::System.Resources.ResourceManager temp = new global::System.Resources.ResourceManager("ThreatModelForge.Analysis.Rules.SensitiveDataToExternalRuleResources", typeof(SensitiveDataToExternalRuleResources).Assembly); + resourceMan = temp; + } + return resourceMan; + } + } + + /// + /// Overrides the current thread's CurrentUICulture property for all + /// resource lookups using this strongly typed resource class. + /// + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Advanced)] + internal static global::System.Globalization.CultureInfo Culture { + get { + return resourceCulture; + } + set { + resourceCulture = value; + } + } + + /// + /// Looks up a localized string similar to the full description of the sensitive-data-to-external rule. + /// + internal static string FullDescription { + get { + return ResourceManager.GetString("FullDescription", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to the help text of the sensitive-data-to-external rule. + /// + internal static string HelpText { + get { + return ResourceManager.GetString("HelpText", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to The {0} sends {1} to an external interactor. + /// + internal static string MessageText { + get { + return ResourceManager.GetString("MessageText", resourceCulture); + } + } + } +} diff --git a/src/ThreatModelForge.Analysis.Rules/SensitiveDataToExternalRuleResources.resx b/src/ThreatModelForge.Analysis.Rules/SensitiveDataToExternalRuleResources.resx new file mode 100644 index 0000000..5c16f30 --- /dev/null +++ b/src/ThreatModelForge.Analysis.Rules/SensitiveDataToExternalRuleResources.resx @@ -0,0 +1,72 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + text/microsoft-resx + + + 2.0 + + + System.Resources.ResXResourceReader, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + + System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + + Sensitive data that flows to an external interactor leaves the system's trust and enters a party the model does not control. Once disclosed, the data cannot be recalled, and the receiving party may retain, forward, or expose it beyond the system's intent. + +This rule fires when a flow whose DataType is a sensitive classification (EUII, EUPI, Customer Content, Account Data, or Access Control Data) has an external interactor as its destination. Flows carrying non-sensitive or public classifications, and flows between internal components, are not reported. A flow flagged here that represents an intended, authorized disclosure can be suppressed. + + + Confirm the external party is authorized to receive data of this classification and that a data-sharing agreement or user consent covers it. Minimize what is sent — redact, tokenize, or aggregate the data so no more than necessary leaves the boundary — and encrypt it in transit. If the disclosure is intended and approved, suppress the finding; otherwise reroute or remove the flow. + + + The {0} sends {1} to an external interactor; confirm the recipient is authorized for this data, minimize what is sent, or remove the flow. + + diff --git a/src/ThreatModelForge.Analysis.Rules/UnauditedBoundaryProcessRule.cs b/src/ThreatModelForge.Analysis.Rules/UnauditedBoundaryProcessRule.cs new file mode 100644 index 0000000..78996d6 --- /dev/null +++ b/src/ThreatModelForge.Analysis.Rules/UnauditedBoundaryProcessRule.cs @@ -0,0 +1,109 @@ +namespace ThreatModelForge.Analysis.Rules +{ + using System; + using System.Linq; + using ThreatModelForge.Model; + using ThreatModelForge.Model.Abstracts; + + /// + /// Rule that flags a process receiving input across a trust boundary that has no reachable audit-log + /// store, so the actions it performs on behalf of callers cannot be attributed after the fact. + /// + /// + /// A process that accepts a data flow crossing a trust boundary is handling requests from a less + /// trusted zone. If none of its outbound flows reach a data store flagged as holding log or audit data + /// (StoresLogData = Yes), there is no durable record of who did what, so a caller can later + /// repudiate an action and an operator cannot reconstruct an incident. + /// + public class UnauditedBoundaryProcessRule : Rule + { + /// + /// Initializes a new instance of the class. + /// + public UnauditedBoundaryProcessRule() + : base(RuleIDs.UnauditedBoundaryProcessRule, MessageSeverity.Warning, RulePackCatalog.StrideCompleteness) + { + this.FullDescription = UnauditedBoundaryProcessRuleResources.FullDescription; + this.HelpText = UnauditedBoundaryProcessRuleResources.HelpText; + this.HelpUri = RuleDocumentation.HelpUriFor(this.ID); + } + + /// + public override IReadOnlyList PropertyBindings => new[] + { + new PropertyBinding("datastore", "StoresLogData"), + }; + + /// + public override StrideCategory? Stride => StrideCategory.Repudiation; + + /// + public override IReadOnlyList ThreatReferences => new[] + { + ThreatReference.Cwe(778), + }; + + /// + public override void Evaluate(RuleEvaluationContext context) + { + _ = context ?? throw new ArgumentNullException(nameof(context)); + + foreach (DrawingSurfaceModel diagram in context.Model.DrawingSurfaceList) + { + foreach (Entity component in diagram.Components()) + { + // Only processes handle requests: skip data stores and external interactors. + if (component.IsStorageComponent() || component.IsExternalInteractor()) + { + continue; + } + + if (!HasInboundTrustBoundaryCrossing(diagram, component)) + { + continue; + } + + if (WritesToAuditLog(diagram, component)) + { + continue; + } + + string text = string.Format( + System.Globalization.CultureInfo.CurrentCulture, + UnauditedBoundaryProcessRuleResources.MessageText, + GetEntityDisplayText(component)); + context.Writer.Write(this.CreateMessage(component, diagram, text)); + } + } + } + + private static bool HasInboundTrustBoundaryCrossing(DrawingSurfaceModel diagram, Entity component) + { + return diagram + .Lines + .Values + .OfType() + .Where(c => c.TargetGuid == component.Guid) + .Any(c => diagram.TrustBoundaryCrossings(c).Any()); + } + + private static bool WritesToAuditLog(DrawingSurfaceModel diagram, Entity component) + { + return diagram + .Lines + .Values + .OfType() + .Where(c => c.SourceGuid == component.Guid) + .Any(c => TargetIsAuditLogStore(diagram, c)); + } + + private static bool TargetIsAuditLogStore(DrawingSurfaceModel diagram, Connector connector) + { + return diagram.Borders.TryGetValue(connector.TargetGuid, out object? value) && + value is Entity target && + target.IsStorageComponent() && + target.TryGetCustomPropertyValue("StoresLogData", out string? storesLogData) && + string.Equals(storesLogData, "Yes", StringComparison.OrdinalIgnoreCase); + } + } +} diff --git a/src/ThreatModelForge.Analysis.Rules/UnauditedBoundaryProcessRuleResources.Designer.cs b/src/ThreatModelForge.Analysis.Rules/UnauditedBoundaryProcessRuleResources.Designer.cs new file mode 100644 index 0000000..3805f56 --- /dev/null +++ b/src/ThreatModelForge.Analysis.Rules/UnauditedBoundaryProcessRuleResources.Designer.cs @@ -0,0 +1,90 @@ +//------------------------------------------------------------------------------ +// +// This code was generated by a tool. +// Runtime Version:4.0.30319.42000 +// +// Changes to this file may cause incorrect behavior and will be lost if +// the code is regenerated. +// +//------------------------------------------------------------------------------ + +namespace ThreatModelForge.Analysis.Rules { + using System; + + + /// + /// A strongly-typed resource class, for looking up localized strings, etc. + /// + // This class was auto-generated by the StronglyTypedResourceBuilder + // class via a tool like ResGen or Visual Studio. + // To add or remove a member, edit your .ResX file then rerun ResGen + // with the /str option, or rebuild your VS project. + [global::System.CodeDom.Compiler.GeneratedCodeAttribute("System.Resources.Tools.StronglyTypedResourceBuilder", "17.0.0.0")] + [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] + [global::System.Runtime.CompilerServices.CompilerGeneratedAttribute()] + internal class UnauditedBoundaryProcessRuleResources { + + private static global::System.Resources.ResourceManager resourceMan; + + private static global::System.Globalization.CultureInfo resourceCulture; + + [global::System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("Microsoft.Performance", "CA1811:AvoidUncalledPrivateCode")] + internal UnauditedBoundaryProcessRuleResources() { + } + + /// + /// Returns the cached ResourceManager instance used by this class. + /// + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Advanced)] + internal static global::System.Resources.ResourceManager ResourceManager { + get { + if (object.ReferenceEquals(resourceMan, null)) { + global::System.Resources.ResourceManager temp = new global::System.Resources.ResourceManager("ThreatModelForge.Analysis.Rules.UnauditedBoundaryProcessRuleResources", typeof(UnauditedBoundaryProcessRuleResources).Assembly); + resourceMan = temp; + } + return resourceMan; + } + } + + /// + /// Overrides the current thread's CurrentUICulture property for all + /// resource lookups using this strongly typed resource class. + /// + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Advanced)] + internal static global::System.Globalization.CultureInfo Culture { + get { + return resourceCulture; + } + set { + resourceCulture = value; + } + } + + /// + /// Looks up a localized string similar to the full description of the unaudited-boundary-process rule. + /// + internal static string FullDescription { + get { + return ResourceManager.GetString("FullDescription", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to the help text of the unaudited-boundary-process rule. + /// + internal static string HelpText { + get { + return ResourceManager.GetString("HelpText", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to The {0} receives input across a trust boundary but writes to no audit-log store. + /// + internal static string MessageText { + get { + return ResourceManager.GetString("MessageText", resourceCulture); + } + } + } +} diff --git a/src/ThreatModelForge.Analysis.Rules/UnauditedBoundaryProcessRuleResources.resx b/src/ThreatModelForge.Analysis.Rules/UnauditedBoundaryProcessRuleResources.resx new file mode 100644 index 0000000..8db7c16 --- /dev/null +++ b/src/ThreatModelForge.Analysis.Rules/UnauditedBoundaryProcessRuleResources.resx @@ -0,0 +1,72 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + text/microsoft-resx + + + 2.0 + + + System.Resources.ResXResourceReader, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + + System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + + A process that accepts input from a less trusted zone acts on behalf of callers whose identity and intent it cannot fully trust. If it keeps no durable audit trail, there is no record to attribute those actions to a caller, so a caller can repudiate what they did and an operator cannot reconstruct an incident. + +This rule fires when a process receiving a data flow that crosses a trust boundary has no outbound flow to a data store flagged as holding log or audit data (StoresLogData = Yes). Processes that do not receive cross-boundary input, and processes that already write to an audit-log store, are not reported. + + + Route the security-relevant actions of the process to a data store that records audit or log data, and mark that store StoresLogData = Yes. Capture who initiated each action, what was requested, and the outcome, so the actions taken on behalf of a caller can be attributed after the fact. Protect the audit store itself (see the unsigned-audit-log and credentials-in-log rules) so the trail cannot be altered or leaked. + + + The {0} receives input across a trust boundary but writes to no audit-log store; route its security-relevant actions to a store marked StoresLogData = Yes so callers' actions can be attributed. + + diff --git a/src/ThreatModelForge.Analysis/README.md b/src/ThreatModelForge.Analysis/README.md index f05bb18..76371fd 100644 --- a/src/ThreatModelForge.Analysis/README.md +++ b/src/ThreatModelForge.Analysis/README.md @@ -1,3 +1,5 @@ # Threat Model Analysis Object Model This project provides the core object model used to inspect threat model (`.tm7`) documents. It exposes the base types that analysis rules build on, so additional rule sets can live in separate assemblies by deriving from those base classes. The `tmforge lint` command runs rules against a model using this library. + +It also projects those findings into the model's persisted **threat register**: a rule that declares a STRIDE category (`Rule.Stride`) becomes a threat via `ThreatGenerator`, which powers the `tmforge threats` command. Validation and threat generation therefore share one detection engine — the difference is lifecycle (a finding is transient and gated; a threat is persisted and triaged). diff --git a/test/ThreatModelForge.Analysis.Rules.Tests/DataStoreMissingBackupRuleTests.cs b/test/ThreatModelForge.Analysis.Rules.Tests/DataStoreMissingBackupRuleTests.cs new file mode 100644 index 0000000..4144f23 --- /dev/null +++ b/test/ThreatModelForge.Analysis.Rules.Tests/DataStoreMissingBackupRuleTests.cs @@ -0,0 +1,112 @@ +namespace ThreatModelForge.Analysis.Rules.Tests +{ + using System; + using Microsoft.VisualStudio.TestTools.UnitTesting; + using ThreatModelForge.KnowledgeBase; + using ThreatModelForge.Model; + using ThreatModelForge.Model.Abstracts; + + /// + /// Unit tests for the class (TM1028). + /// + [TestClass] + public class DataStoreMissingBackupRuleTests + { + private const string StorageComponentGenericTypeId = "GE.DS"; + + /// + /// Verifies the rule's identity and populated metadata. + /// + [TestMethod] + public void ConstructorTest() + { + using DataStoreMissingBackupRule target = new DataStoreMissingBackupRule(); + Assert.AreEqual("TM1028", target.ID); + Assert.AreEqual(MessageSeverity.Warning, target.Severity); + Assert.IsNotNull(target.HelpUri); + Assert.IsFalse(string.IsNullOrEmpty(target.FullDescription)); + Assert.IsFalse(string.IsNullOrEmpty(target.HelpText)); + } + + /// + /// A credential store with Backup = No is flagged. + /// + [TestMethod] + public void FlagsCredentialStoreWithoutBackup() + { + StencilParallelLines store = CreateStore("Vault", ("StoresCredentials", "Yes"), ("Backup", "No")); + MockMessageWriter writer = Evaluate(store); + + Assert.AreEqual(1, writer.Messages.Count); + Assert.AreSame(store, writer.Messages[0].Target); + Assert.IsTrue(writer.Messages[0].Text!.Contains("Vault")); + } + + /// + /// A log store that never declares Backup is flagged (unset is treated as no backup). + /// + [TestMethod] + public void FlagsLogStoreWithoutBackupProperty() + { + StencilParallelLines store = CreateStore("Audit Log", ("StoresLogData", "Yes")); + MockMessageWriter writer = Evaluate(store); + + Assert.AreEqual(1, writer.Messages.Count); + } + + /// + /// A credential store that is backed up is not flagged. + /// + [TestMethod] + public void IgnoresBackedUpCredentialStore() + { + StencilParallelLines store = CreateStore("Vault", ("StoresCredentials", "Yes"), ("Backup", "Yes")); + MockMessageWriter writer = Evaluate(store); + + Assert.AreEqual(0, writer.Messages.Count); + } + + /// + /// A store that holds no important data is not flagged even without a backup. + /// + [TestMethod] + public void IgnoresUnimportantStoreWithoutBackup() + { + StencilParallelLines store = CreateStore("Scratch Cache", ("Backup", "No")); + MockMessageWriter writer = Evaluate(store); + + Assert.AreEqual(0, writer.Messages.Count); + } + + private static MockMessageWriter Evaluate(Entity store) + { + DrawingSurfaceModel diagram = new DrawingSurfaceModel { Header = "DFD-0" }; + diagram.Borders.Add(store.Guid, store); + ThreatModel model = new ThreatModel { DrawingSurfaceList = { diagram } }; + + MockMessageWriter writer = new MockMessageWriter(); + using (DataStoreMissingBackupRule target = new DataStoreMissingBackupRule()) + { + target.Evaluate(new RuleEvaluationContext(model, writer)); + } + + return writer; + } + + private static StencilParallelLines CreateStore(string name, params (string Name, string Value)[] properties) + { + StencilParallelLines store = new StencilParallelLines + { + Guid = Guid.NewGuid(), + GenericTypeId = StorageComponentGenericTypeId, + }; + store.Properties.Add(new StringDisplayAttribute { DisplayName = "Name", Value = name }); + foreach ((string propertyName, string propertyValue) in properties) + { + store.Properties.Add(new CustomStringDisplayAttribute { Value = $"{propertyName}:{propertyValue}" }); + } + + return store; + } + } +} diff --git a/test/ThreatModelForge.Analysis.Rules.Tests/SensitiveDataToExternalRuleTests.cs b/test/ThreatModelForge.Analysis.Rules.Tests/SensitiveDataToExternalRuleTests.cs new file mode 100644 index 0000000..ef280c7 --- /dev/null +++ b/test/ThreatModelForge.Analysis.Rules.Tests/SensitiveDataToExternalRuleTests.cs @@ -0,0 +1,139 @@ +namespace ThreatModelForge.Analysis.Rules.Tests +{ + using System; + using Microsoft.VisualStudio.TestTools.UnitTesting; + using ThreatModelForge.KnowledgeBase; + using ThreatModelForge.Model; + + /// + /// Unit tests for the class (TM1030). + /// + [TestClass] + public class SensitiveDataToExternalRuleTests + { + private const string ProcessGenericTypeId = "GE.P"; + + private const string ExternalInteractorGenericTypeId = "GE.EI"; + + /// + /// Verifies the rule's identity and populated metadata. + /// + [TestMethod] + public void ConstructorTest() + { + using SensitiveDataToExternalRule target = new SensitiveDataToExternalRule(); + Assert.AreEqual("TM1030", target.ID); + Assert.AreEqual(MessageSeverity.Warning, target.Severity); + Assert.IsNotNull(target.HelpUri); + Assert.IsFalse(string.IsNullOrEmpty(target.FullDescription)); + Assert.IsFalse(string.IsNullOrEmpty(target.HelpText)); + } + + /// + /// A flow carrying a sensitive classification to an external interactor is flagged. + /// + [TestMethod] + public void FlagsSensitiveFlowToExternal() + { + StencilRectangle external = CreateExternal("Partner API"); + Connector edge = EdgeTo(external.Guid, ("DataType", "EUII")); + MockMessageWriter writer = Evaluate(external, edge); + + Assert.AreEqual(1, writer.Messages.Count); + Assert.AreSame(edge, writer.Messages[0].Target); + Assert.IsTrue(writer.Messages[0].Text!.Contains("EUII")); + } + + /// + /// A flow carrying a non-sensitive classification to an external interactor is not flagged. + /// + [TestMethod] + public void IgnoresNonSensitiveFlowToExternal() + { + StencilRectangle external = CreateExternal("Partner API"); + Connector edge = EdgeTo(external.Guid, ("DataType", "Public Non-Personal Data")); + MockMessageWriter writer = Evaluate(external, edge); + + Assert.AreEqual(0, writer.Messages.Count); + } + + /// + /// A flow with no data classification to an external interactor is not flagged. + /// + [TestMethod] + public void IgnoresUnclassifiedFlowToExternal() + { + StencilRectangle external = CreateExternal("Partner API"); + Connector edge = EdgeTo(external.Guid); + MockMessageWriter writer = Evaluate(external, edge); + + Assert.AreEqual(0, writer.Messages.Count); + } + + /// + /// A sensitive flow whose destination is an internal component is not flagged. + /// + [TestMethod] + public void IgnoresSensitiveFlowToInternalComponent() + { + StencilEllipse process = new StencilEllipse + { + Guid = Guid.NewGuid(), + GenericTypeId = ProcessGenericTypeId, + }; + process.Properties.Add(new StringDisplayAttribute { DisplayName = "Name", Value = "Billing" }); + Connector edge = EdgeTo(process.Guid, ("DataType", "Customer Content")); + + DrawingSurfaceModel diagram = new DrawingSurfaceModel { Header = "DFD-0" }; + diagram.Borders.Add(process.Guid, process); + diagram.Lines.Add(edge.Guid, edge); + ThreatModel model = new ThreatModel { DrawingSurfaceList = { diagram } }; + + MockMessageWriter writer = new MockMessageWriter(); + using (SensitiveDataToExternalRule target = new SensitiveDataToExternalRule()) + { + target.Evaluate(new RuleEvaluationContext(model, writer)); + } + + Assert.AreEqual(0, writer.Messages.Count); + } + + private static MockMessageWriter Evaluate(StencilRectangle external, Connector edge) + { + DrawingSurfaceModel diagram = new DrawingSurfaceModel { Header = "DFD-0" }; + diagram.Borders.Add(external.Guid, external); + diagram.Lines.Add(edge.Guid, edge); + ThreatModel model = new ThreatModel { DrawingSurfaceList = { diagram } }; + + MockMessageWriter writer = new MockMessageWriter(); + using (SensitiveDataToExternalRule target = new SensitiveDataToExternalRule()) + { + target.Evaluate(new RuleEvaluationContext(model, writer)); + } + + return writer; + } + + private static StencilRectangle CreateExternal(string name) + { + StencilRectangle external = new StencilRectangle + { + Guid = Guid.NewGuid(), + GenericTypeId = ExternalInteractorGenericTypeId, + }; + external.Properties.Add(new StringDisplayAttribute { DisplayName = "Name", Value = name }); + return external; + } + + private static Connector EdgeTo(Guid target, params (string Name, string Value)[] properties) + { + Connector edge = new Connector { Guid = Guid.NewGuid(), SourceGuid = Guid.NewGuid(), TargetGuid = target }; + foreach ((string propertyName, string propertyValue) in properties) + { + edge.Properties.Add(new CustomStringDisplayAttribute { Value = $"{propertyName}:{propertyValue}" }); + } + + return edge; + } + } +} diff --git a/test/ThreatModelForge.Analysis.Rules.Tests/UnauditedBoundaryProcessRuleTests.cs b/test/ThreatModelForge.Analysis.Rules.Tests/UnauditedBoundaryProcessRuleTests.cs new file mode 100644 index 0000000..85e4f72 --- /dev/null +++ b/test/ThreatModelForge.Analysis.Rules.Tests/UnauditedBoundaryProcessRuleTests.cs @@ -0,0 +1,197 @@ +namespace ThreatModelForge.Analysis.Rules.Tests +{ + using System; + using Microsoft.VisualStudio.TestTools.UnitTesting; + using ThreatModelForge.KnowledgeBase; + using ThreatModelForge.Model; + + /// + /// Unit tests for the class (TM1029). + /// + [TestClass] + public class UnauditedBoundaryProcessRuleTests + { + private const string ProcessGenericTypeId = "GE.P"; + + private const string StorageComponentGenericTypeId = "GE.DS"; + + /// + /// Verifies the rule's identity and populated metadata. + /// + [TestMethod] + public void ConstructorTest() + { + using UnauditedBoundaryProcessRule target = new UnauditedBoundaryProcessRule(); + Assert.AreEqual("TM1029", target.ID); + Assert.AreEqual(MessageSeverity.Warning, target.Severity); + Assert.IsNotNull(target.HelpUri); + Assert.IsFalse(string.IsNullOrEmpty(target.FullDescription)); + Assert.IsFalse(string.IsNullOrEmpty(target.HelpText)); + } + + /// + /// A boundary-facing process with no reachable audit-log store is flagged. + /// + [TestMethod] + public void FlagsBoundaryProcessWithoutAuditLog() + { + StencilEllipse process = CreateProcess("Order Service"); + BorderBoundary border = CreateBoundary(); + Connector inbound = CrossingInbound(process); + ThreatModel model = BuildModel(border, process, inbound); + + MockMessageWriter writer = Evaluate(model); + + Assert.AreEqual(1, writer.Messages.Count); + Assert.AreSame(process, writer.Messages[0].Target); + Assert.IsTrue(writer.Messages[0].Text!.Contains("Order Service")); + } + + /// + /// A boundary-facing process that writes to an audit-log store is not flagged. + /// + [TestMethod] + public void IgnoresBoundaryProcessThatWritesToAuditLog() + { + StencilEllipse process = CreateProcess("Order Service"); + BorderBoundary border = CreateBoundary(); + StencilParallelLines log = CreateStore("Audit Log", ("StoresLogData", "Yes")); + Connector inbound = CrossingInbound(process); + Connector toLog = Edge(process.Guid, log.Guid); + ThreatModel model = BuildModel(border, process, inbound, log, toLog); + + MockMessageWriter writer = Evaluate(model); + + Assert.AreEqual(0, writer.Messages.Count); + } + + /// + /// A boundary-facing process that writes only to a non-log store is still flagged. + /// + [TestMethod] + public void FlagsBoundaryProcessWritingOnlyToNonLogStore() + { + StencilEllipse process = CreateProcess("Order Service"); + BorderBoundary border = CreateBoundary(); + StencilParallelLines store = CreateStore("Orders", ("StoresLogData", "No")); + Connector inbound = CrossingInbound(process); + Connector toStore = Edge(process.Guid, store.Guid); + ThreatModel model = BuildModel(border, process, inbound, store, toStore); + + MockMessageWriter writer = Evaluate(model); + + Assert.AreEqual(1, writer.Messages.Count); + } + + /// + /// A process whose inbound edge does not cross a trust boundary is not flagged. + /// + [TestMethod] + public void IgnoresProcessWithoutCrossBoundaryInput() + { + StencilEllipse process = CreateProcess("Order Service"); + BorderBoundary border = CreateBoundary(); + Connector inbound = NonCrossingInbound(process); + ThreatModel model = BuildModel(border, process, inbound); + + MockMessageWriter writer = Evaluate(model); + + Assert.AreEqual(0, writer.Messages.Count); + } + + private static MockMessageWriter Evaluate(ThreatModel model) + { + MockMessageWriter writer = new MockMessageWriter(); + using (UnauditedBoundaryProcessRule target = new UnauditedBoundaryProcessRule()) + { + target.Evaluate(new RuleEvaluationContext(model, writer)); + } + + return writer; + } + + private static StencilEllipse CreateProcess(string name) + { + StencilEllipse process = new StencilEllipse + { + Guid = Guid.NewGuid(), + GenericTypeId = ProcessGenericTypeId, + }; + process.Properties.Add(new StringDisplayAttribute { DisplayName = "Name", Value = name }); + return process; + } + + private static StencilParallelLines CreateStore(string name, params (string Name, string Value)[] properties) + { + StencilParallelLines store = new StencilParallelLines + { + Guid = Guid.NewGuid(), + GenericTypeId = StorageComponentGenericTypeId, + }; + store.Properties.Add(new StringDisplayAttribute { DisplayName = "Name", Value = name }); + foreach ((string propertyName, string propertyValue) in properties) + { + store.Properties.Add(new CustomStringDisplayAttribute { Value = $"{propertyName}:{propertyValue}" }); + } + + return store; + } + + private static BorderBoundary CreateBoundary() + { + return new BorderBoundary { Guid = Guid.NewGuid(), Left = 5, Top = 10, Height = 15, Width = 20 }; + } + + private static Connector CrossingInbound(StencilEllipse process) + { + return new Connector + { + Guid = Guid.NewGuid(), + SourceX = 0, + SourceY = 0, + TargetX = 10, + TargetY = 11, + TargetGuid = process.Guid, + }; + } + + private static Connector NonCrossingInbound(StencilEllipse process) + { + return new Connector + { + Guid = Guid.NewGuid(), + SourceX = 8, + SourceY = 12, + TargetX = 10, + TargetY = 11, + TargetGuid = process.Guid, + }; + } + + private static Connector Edge(Guid source, Guid target) + { + return new Connector { Guid = Guid.NewGuid(), SourceGuid = source, TargetGuid = target }; + } + + private static ThreatModel BuildModel(BorderBoundary border, StencilEllipse process, params object[] elements) + { + DrawingSurfaceModel diagram = new DrawingSurfaceModel { Header = "DFD-0" }; + diagram.Borders.Add(border.Guid, border); + diagram.Borders.Add(process.Guid, process); + foreach (object element in elements) + { + switch (element) + { + case Connector connector: + diagram.Lines.Add(connector.Guid, connector); + break; + case StencilParallelLines store: + diagram.Borders.Add(store.Guid, store); + break; + } + } + + return new ThreatModel { DrawingSurfaceList = { diagram } }; + } + } +} diff --git a/test/ThreatModelForge.Api.Tests/RuleCatalogTest.cs b/test/ThreatModelForge.Api.Tests/RuleCatalogTest.cs index a0b4605..e7fe522 100644 --- a/test/ThreatModelForge.Api.Tests/RuleCatalogTest.cs +++ b/test/ThreatModelForge.Api.Tests/RuleCatalogTest.cs @@ -20,6 +20,7 @@ public class RuleCatalogTest "data-protection", "transport-security", "identity-access", + "availability", }; private static readonly HashSet ValidSeverities = new HashSet(StringComparer.Ordinal) @@ -123,7 +124,7 @@ public void GetRulePacks_AreInPresentationOrder() { List ids = EngineService.GetRulePacks().Select(pack => pack.Id).ToList(); CollectionAssert.AreEqual( - new[] { "core-hygiene", "stride-completeness", "input-validation", "data-protection", "transport-security", "identity-access" }, + new[] { "core-hygiene", "stride-completeness", "input-validation", "data-protection", "transport-security", "identity-access", "availability" }, ids, "Rule packs must be returned in presentation order."); } From 231f0072ddfcb066bae185ceb2c4d04486221038 Mon Sep 17 00:00:00 2001 From: Hacks4Snacks Date: Wed, 8 Jul 2026 00:12:01 -0500 Subject: [PATCH 3/8] Studio support for threat generation --- docs/validation-rules.md | 14 +- src/ThreatModelForge.Api/Program.cs | 3 + src/ThreatModelForge.Api/openapi/v1.json | 115 ++++++++ src/ThreatModelForge.Engine/EngineService.cs | 24 ++ src/ThreatModelForge.Engine/ThreatDto.cs | 6 + src/ThreatModelForge.Engine/ThreatStateDto.cs | 20 ++ .../TmForgeModelDto.cs | 7 + src/ThreatModelForge.Studio/src/App.css | 178 +++++++++++++ .../src/dfd/Editor.tsx | 164 +++++++----- .../src/dfd/ThreatsPanel.test.tsx | 166 ++++++++++++ .../src/dfd/ThreatsPanel.tsx | 252 ++++++++++++++++++ .../src/dfd/Toolbar.tsx | 10 +- .../src/dfd/engine/schema.d.ts | 60 +++++ .../src/dfd/engineClient.ts | 74 +++++ src/ThreatModelForge.Studio/src/dfd/types.ts | 15 ++ src/ThreatModelForge.Wasm/Engine.cs | 7 + .../EngineGenerateThreatsTest.cs | 36 ++- 17 files changed, 1072 insertions(+), 79 deletions(-) create mode 100644 src/ThreatModelForge.Engine/ThreatStateDto.cs create mode 100644 src/ThreatModelForge.Studio/src/dfd/ThreatsPanel.test.tsx create mode 100644 src/ThreatModelForge.Studio/src/dfd/ThreatsPanel.tsx diff --git a/docs/validation-rules.md b/docs/validation-rules.md index d6da2f8..a66f29c 100644 --- a/docs/validation-rules.md +++ b/docs/validation-rules.md @@ -42,6 +42,7 @@ that declare them. List them from the API with `GET /v1/rule-packs`. | `data-protection` | Data Protection | Data-at-rest protection: encryption, access control, integrity, retention. | | `transport-security` | Transport Security | Data-in-transit protection across trust boundaries. | | `identity-access` | Identity & Access | Authentication, least privilege, and access to components. | +| `availability` | Availability | Recoverability and audit-trail durability: backups for important data. | ## Built-in rules @@ -72,6 +73,7 @@ snapshot. `tmforge` and the engine's `GET /v1/rules` report the live rule set. | 1009 | Edge missing protocol description | Info | A flow mentions its protocol in the description text. | | 1010 | Edge missing port | Warning | A flow declares a port when it can't be inferred from the protocol. | | 1013 | Edge missing data classification | Warning | A flow declares a data classification. | +| 1029 | Unaudited boundary process | Warning | A process receiving input across a trust boundary writes to an audit-log store so its actions can be attributed. | ### Input Validation (`input-validation`) @@ -91,6 +93,7 @@ snapshot. `tmforge` and the engine's `GET /v1/rules` report the live rule set. | 1022 | Credentials in log store | Warning | A store recording log data does not also store credentials. | | 1025 | Weak or unapproved cipher | Warning | A flow or store declaring an encryption algorithm uses an approved authenticated cipher (AES-GCM, AES-CBC+HMAC, or ChaCha20-Poly1305). | | 1027 | Cached credential read | Warning | A flow reading from a credential store is not cached (`Cached=No`), so a rotated or revoked credential is not served stale. | +| 1030 | Sensitive data to external | Warning | A flow carrying sensitive data (EUII, EUPI, customer content, account data, or access-control data) is not sent to an external interactor. | ### Transport Security (`transport-security`) @@ -107,6 +110,12 @@ snapshot. `tmforge` and the engine's `GET /v1/rules` report the live rule set. | 1024 | Over-privileged process | Warning | A process does not run as a highly privileged account (root/admin/system). | | 1026 | Shared static identity | Warning | A single `Identity` is not asserted by flows from 2+ distinct sources; each calling principal has its own scoped identity. | +### Availability (`availability`) + +| ID | Rule | Severity | Checks | +| --- | --- | --- | --- | +| 1028 | Data store without backup | Warning | A store holding credentials or audit/log data declares a backup (`Backup=Yes`), so its contents can be recovered after loss or a destructive attack. | + ## Rule help Every finding carries the rule's **ID** (for example `TM1016`). To see what a rule checks and how to @@ -136,8 +145,9 @@ tmforge set model.tm7 --id --property AuthenticationScheme=OAuth Common rule-checked properties include `Protocol`, `Port`, `DataType` / data classification, `AuthenticationScheme`, `SanitizesInput` / `SanitizesOutput`, `Isolation`, `AccessControl`, `Signed`, -`AuthenticatesItself`, `RunningAs`, `Algorithm`, and encryption/at-rest flags. In -[Studio](studio-guide.md), edit the same properties in the inspector and re-**Validate**. +`AuthenticatesItself`, `RunningAs`, `Algorithm`, `StoresCredentials` / `StoresLogData`, `Backup`, and +encryption/at-rest flags. In [Studio](studio-guide.md), edit the same properties in the inspector and +re-**Validate**. ## Customizing the rule set diff --git a/src/ThreatModelForge.Api/Program.cs b/src/ThreatModelForge.Api/Program.cs index bb03d63..4cbd218 100644 --- a/src/ThreatModelForge.Api/Program.cs +++ b/src/ThreatModelForge.Api/Program.cs @@ -65,6 +65,9 @@ public static void Main(string[] args) app.MapPost("/v1/model/validate", (TmForgeModelDto model) => TypedResults.Ok(EngineService.Validate(model))) .WithName("ValidateModel") .WithTags("Model"); + app.MapPost("/v1/model/threats", (TmForgeModelDto model) => TypedResults.Ok(EngineService.GenerateThreats(model))) + .WithName("GenerateThreats") + .WithTags("Model"); app.MapPost( "/v1/model/merge", (MergeRequestDto request) => TypedResults.Ok( diff --git a/src/ThreatModelForge.Api/openapi/v1.json b/src/ThreatModelForge.Api/openapi/v1.json index ca104f2..5e4f28d 100644 --- a/src/ThreatModelForge.Api/openapi/v1.json +++ b/src/ThreatModelForge.Api/openapi/v1.json @@ -196,6 +196,39 @@ } } }, + "/v1/model/threats": { + "post": { + "tags": [ + "Model" + ], + "operationId": "GenerateThreats", + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/TmForgeModelDto" + } + } + }, + "required": true + }, + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "type": "array", + "items": { + "$ref": "#/components/schemas/ThreatDto" + } + } + } + } + } + } + } + }, "/v1/model/merge": { "post": { "tags": [ @@ -711,6 +744,79 @@ }, "description": "Describes an authoring stencil: a named, categorized specialization of one of the four DFD\nprimitives (`process`, `datastore`, `external`, `boundary`). A stencil\nmaps to its string StencilDto.Base primitive for analysis, and its identity plus preset\nproperties enrich the model without requiring per-stencil rules." }, + "ThreatDto": { + "type": "object", + "properties": { + "id": { + "type": "string" + }, + "ruleId": { + "type": "string" + }, + "category": { + "type": "string" + }, + "title": { + "type": "string" + }, + "mitigation": { + "type": [ + "null", + "string" + ] + }, + "severity": { + "type": "string" + }, + "priority": { + "type": [ + "null", + "string" + ] + }, + "references": { + "type": "array", + "items": { + "type": "string" + } + }, + "elementIds": { + "type": "array", + "items": { + "type": "string" + } + }, + "interaction": { + "type": "string" + }, + "state": { + "type": "string" + }, + "justification": { + "type": [ + "null", + "string" + ] + } + } + }, + "ThreatStateDto": { + "type": "object", + "properties": { + "id": { + "type": "string" + }, + "state": { + "type": "string" + }, + "justification": { + "type": [ + "null", + "string" + ] + } + } + }, "TmForgeDiagramDto": { "type": "object", "properties": { @@ -874,6 +980,15 @@ "$ref": "#/components/schemas/TmForgeValidationDto" } ] + }, + "threats": { + "type": [ + "null", + "array" + ], + "items": { + "$ref": "#/components/schemas/ThreatStateDto" + } } } }, diff --git a/src/ThreatModelForge.Engine/EngineService.cs b/src/ThreatModelForge.Engine/EngineService.cs index 9c14e39..7711b17 100644 --- a/src/ThreatModelForge.Engine/EngineService.cs +++ b/src/ThreatModelForge.Engine/EngineService.cs @@ -201,8 +201,10 @@ public static IReadOnlyList GenerateThreats(TmForgeModelDto dto) } GenerationResult generation = ThreatGenerator.Generate(model, ruleSet); + Dictionary triage = BuildTriage(dto.Threats); foreach (GeneratedThreat threat in generation.Threats) { + triage.TryGetValue(threat.Id, out ThreatStateDto? state); result.Add(new ThreatDto { Id = threat.Id, @@ -215,6 +217,8 @@ public static IReadOnlyList GenerateThreats(TmForgeModelDto dto) References = threat.References.Select(r => r.Id).ToList(), ElementIds = BuildElementIds(threat), Interaction = threat.InteractionString, + State = NormalizeState(state?.State), + Justification = state?.Justification, }); } } @@ -393,6 +397,26 @@ private static IReadOnlyList BuildElementIds(GeneratedThreat threat) return ids; } + private static Dictionary BuildTriage(IReadOnlyList? states) + { + Dictionary map = new Dictionary(StringComparer.OrdinalIgnoreCase); + if (states != null) + { + foreach (ThreatStateDto state in states) + { + if (!string.IsNullOrEmpty(state.Id)) + { + map[state.Id] = state; + } + } + } + + return map; + } + + private static string NormalizeState(string? state) + => string.Equals(state, "Accepted", StringComparison.OrdinalIgnoreCase) ? "Accepted" : "Open"; + private static ThreatModel BuildModel(TmForgeModelDto dto, out Dictionary> nameToIds) { nameToIds = new Dictionary>(StringComparer.Ordinal); diff --git a/src/ThreatModelForge.Engine/ThreatDto.cs b/src/ThreatModelForge.Engine/ThreatDto.cs index f15d106..b3cc0d8 100644 --- a/src/ThreatModelForge.Engine/ThreatDto.cs +++ b/src/ThreatModelForge.Engine/ThreatDto.cs @@ -37,5 +37,11 @@ public sealed class ThreatDto /// Gets the human-readable scope string (source -> target for a flow, else the element name). public string Interaction { get; init; } = string.Empty; + + /// Gets the triage state (Open or Accepted). + public string State { get; init; } = "Open"; + + /// Gets the risk-acceptance justification, when the threat has been accepted. + public string? Justification { get; init; } } } diff --git a/src/ThreatModelForge.Engine/ThreatStateDto.cs b/src/ThreatModelForge.Engine/ThreatStateDto.cs new file mode 100644 index 0000000..4820e81 --- /dev/null +++ b/src/ThreatModelForge.Engine/ThreatStateDto.cs @@ -0,0 +1,20 @@ +namespace ThreatModelForge.Engine +{ + /// + /// A triage-overlay entry: the persisted lifecycle state of one generated threat, carried on the + /// model so risk acceptance round-trips with it. Keyed by the threat's register id + /// ({targetGuid:N}:{ruleId}). Only threats whose state differs from the default + /// (Open) need an entry. + /// + public sealed class ThreatStateDto + { + /// Gets the register id of the threat this state applies to. + public string Id { get; init; } = string.Empty; + + /// Gets the triage state (Open or Accepted). + public string State { get; init; } = "Open"; + + /// Gets the risk-acceptance justification, when the threat has been accepted. + public string? Justification { get; init; } + } +} diff --git a/src/ThreatModelForge.Engine/TmForgeModelDto.cs b/src/ThreatModelForge.Engine/TmForgeModelDto.cs index 535cd6c..bc6e812 100644 --- a/src/ThreatModelForge.Engine/TmForgeModelDto.cs +++ b/src/ThreatModelForge.Engine/TmForgeModelDto.cs @@ -27,5 +27,12 @@ public sealed class TmForgeModelDto /// Gets the per-model validation configuration (which rule packs or rules to skip). public TmForgeValidationDto? Validation { get; init; } + + /// + /// Gets the threat triage overlay: the persisted lifecycle state (accepted risks and their + /// justifications) of generated threats, keyed by threat id. Absent or empty means every + /// threat is open. + /// + public IReadOnlyList? Threats { get; init; } } } diff --git a/src/ThreatModelForge.Studio/src/App.css b/src/ThreatModelForge.Studio/src/App.css index 6a23310..0dbe096 100644 --- a/src/ThreatModelForge.Studio/src/App.css +++ b/src/ThreatModelForge.Studio/src/App.css @@ -954,6 +954,184 @@ margin-right: 2px; } +/* ---- threats panel (STRIDE register) ---- */ +.threats { + background: var(--surface); + border: 1px solid var(--border); + border-radius: 10px; + padding: 10px 12px; + max-width: 340px; + max-height: 50vh; + overflow-y: auto; + box-shadow: 0 8px 24px rgba(15, 23, 42, 0.14); + font-size: 12px; + margin-top: 8px; +} + +.threats h3 { + margin: 0 0 6px; + font-size: 12px; +} + +.threat-group { + margin-bottom: 6px; +} + +.threat-group-head { + display: flex; + align-items: center; + gap: 6px; + font-weight: 700; + font-size: 11px; + text-transform: uppercase; + letter-spacing: 0.02em; + color: var(--muted); + padding: 4px 0 2px; + border-bottom: 1px solid var(--border); + margin-bottom: 2px; +} + +.threat-count { + font-weight: 700; + color: var(--muted); + background: var(--surface-3); + border-radius: 6px; + padding: 0 6px; + font-size: 10px; +} + +.threat { + display: flex; + flex-direction: column; + padding: 4px 0; +} + +.threat-row { + display: flex; + gap: 8px; + align-items: flex-start; +} + +.threat-head { + display: flex; + gap: 8px; + flex: 1 1 auto; + min-width: 0; + line-height: 1.35; + cursor: pointer; +} + +.threat-head .sev { + font-weight: 700; + text-transform: uppercase; + font-size: 10px; + flex: 0 0 auto; + padding-top: 2px; +} + +.threat-actions { + flex: 0 0 auto; +} + +.threat-action { + font-size: 11px; + font-weight: 600; + background: none; + border: none; + color: #2563eb; + cursor: pointer; + padding: 1px 4px; + white-space: nowrap; +} + +.threat-action:hover { + text-decoration: underline; +} + +.threat-accepted { + opacity: 0.68; +} + +.threat-badge { + font-size: 10px; + font-weight: 700; + text-transform: uppercase; + color: #047857; + background: #ecfdf5; + border-radius: 4px; + padding: 0 5px; + margin-left: 4px; +} + +.threat-justification { + font-size: 11px; + font-style: italic; + color: var(--muted); + margin-top: 2px; +} + +.threat-accept { + display: flex; + flex-direction: column; + gap: 6px; + margin: 6px 0 4px 28px; +} + +.threat-accept-input { + width: 100%; + min-height: 44px; + font: inherit; + font-size: 12px; + padding: 6px; + border: 1px solid var(--border); + border-radius: 6px; + resize: vertical; + box-sizing: border-box; +} + +.threat-accept-buttons { + display: flex; + gap: 6px; +} + +.threats-accepted-count { + color: var(--muted); + font-weight: 400; +} + +.threat-body { + display: flex; + flex-direction: column; + gap: 2px; + min-width: 0; +} + +.threat-meta { + display: flex; + flex-wrap: wrap; + align-items: center; + gap: 6px; +} + +.threat-scope { + color: var(--muted); + font-size: 11px; +} + +.threat-ref { + font-size: 10px; + font-weight: 700; + color: #0e7490; + background: #ecfeff; + border-radius: 4px; + padding: 0 5px; + text-decoration: none; +} + +a.threat-ref:hover { + text-decoration: underline; +} + /* engine status pill */ .engine-pill { display: inline-flex; diff --git a/src/ThreatModelForge.Studio/src/dfd/Editor.tsx b/src/ThreatModelForge.Studio/src/dfd/Editor.tsx index 916cbe5..bf474c7 100644 --- a/src/ThreatModelForge.Studio/src/dfd/Editor.tsx +++ b/src/ThreatModelForge.Studio/src/dfd/Editor.tsx @@ -28,7 +28,8 @@ import { Toolbar } from './Toolbar'; import { Inspector } from './Inspector'; import { ValidationSettings } from './ValidationSettings'; import { FALLBACK_PACKS, FALLBACK_STENCILS } from './stencils'; -import { createHttpEngine, loadWasmEngine, offlineEngine, probeEngine, type Finding, type FormatInfo, type IEngineClient, type PackInfo, type PropertyDescriptorInfo, type RuleInfo, type RulePackInfo, type StencilInfo } from './engineClient'; +import { createHttpEngine, loadWasmEngine, offlineEngine, probeEngine, type Finding, type FormatInfo, type IEngineClient, type PackInfo, type PropertyDescriptorInfo, type RuleInfo, type RulePackInfo, type StencilInfo, type Threat } from './engineClient'; +import { ThreatsPanel } from './ThreatsPanel'; import { DEFAULT_NODE_SIZE, modelFromPages, pagesFromModel, type PageGraph } from './mapping'; import { useUndoRedo } from './useUndoRedo'; import { FlowEdge } from './edges/FlowEdge'; @@ -36,7 +37,7 @@ import { PageTabs } from './PageTabs'; import { MergeResolveModal } from './MergeResolveModal'; import { DfdActionsContext, type DfdActions } from './editorContext'; import { Toaster, toast } from './toast'; -import type { DfdEdge, DfdKind, DfdNode, TmForgeModel, TmForgeValidation } from './types'; +import type { DfdEdge, DfdKind, DfdNode, ThreatTriage, TmForgeModel, TmForgeValidation } from './types'; const nodeTypes: NodeTypes = { process: ShapeNode, @@ -89,6 +90,7 @@ interface StoredWorkspace { pages: PageGraph[]; activePageId: string; validation?: TmForgeValidation; + threats?: ThreatTriage[]; } /** Reads the saved multi-page workspace (v2), migrating a legacy single-page model (v1) when present. */ @@ -100,7 +102,7 @@ function loadStoredWorkspace(): StoredWorkspace | null { if (parsed?.model?.schema === 'tmforge-json') { const pages = pagesFromModel(parsed.model); const activePageId = pages.some((p) => p.id === parsed.activePageId) ? parsed.activePageId! : pages[0].id; - return { pages, activePageId, validation: parsed.model.validation }; + return { pages, activePageId, validation: parsed.model.validation, threats: parsed.model.threats }; } } } catch { @@ -112,7 +114,7 @@ function loadStoredWorkspace(): StoredWorkspace | null { const model = JSON.parse(raw) as TmForgeModel; if (model?.schema === 'tmforge-json') { const pages = pagesFromModel(model); - return { pages, activePageId: pages[0].id, validation: model.validation }; + return { pages, activePageId: pages[0].id, validation: model.validation, threats: model.threats }; } } } catch { @@ -232,6 +234,8 @@ export function Editor() { const [nodes, setNodes, onNodesChange] = useNodesState(INITIAL_ACTIVE.nodes); const [edges, setEdges, onEdgesChange] = useEdgesState(INITIAL_ACTIVE.edges); const [findings, setFindings] = useState([]); + const [threats, setThreats] = useState([]); + const [threatTriage, setThreatTriage] = useState(INITIAL_WORKSPACE.threats ?? []); const flaggedIdsRef = useRef>(new Set()); const [engine, setEngine] = useState(offlineEngine); const [engineOnline, setEngineOnline] = useState(false); @@ -419,10 +423,10 @@ export function Editor() { // the model, so selecting a node never marks it dirty); `dirty` compares it to the snapshot from // the last explicit Save. A debounced localStorage write of the whole workspace (pages + active // tab) runs on every change as a crash-recovery net, so a reload never loses work. - const currentModel = useMemo( - () => modelFromPages(allPages, buildValidation(disabledRulePacks, disabledRuleIds)), - [allPages, disabledRulePacks, disabledRuleIds], - ); + const currentModel = useMemo(() => { + const model = modelFromPages(allPages, buildValidation(disabledRulePacks, disabledRuleIds)); + return threatTriage.length > 0 ? { ...model, threats: threatTriage } : model; + }, [allPages, disabledRulePacks, disabledRuleIds, threatTriage]); const currentJson = useMemo(() => JSON.stringify(currentModel), [currentModel]); const [savedJson, setSavedJson] = useState(INITIAL_SAVED_JSON); const dirty = currentJson !== savedJson; @@ -543,8 +547,9 @@ export function Editor() { const findingPageIds = useMemo(() => { const set = new Set(); - for (const finding of findings) { - for (const id of finding.elementIds) { + const elementIdLists = [...findings.map((f) => f.elementIds), ...threats.map((t) => t.elementIds)]; + for (const elementIds of elementIdLists) { + for (const id of elementIds) { const pageId = elementPageIndex.get(id); if (pageId) { set.add(pageId); @@ -552,20 +557,7 @@ export function Editor() { } } return set; - }, [findings, elementPageIndex]); - - const jumpToFinding = useCallback( - (finding: Finding) => { - for (const id of finding.elementIds) { - const pageId = elementPageIndex.get(id); - if (pageId && pageId !== activePageId) { - switchPage(pageId); - return; - } - } - }, - [elementPageIndex, activePageId, switchPage], - ); + }, [findings, threats, elementPageIndex]); const serializeModel = useCallback( async (formatId: string): Promise => { @@ -878,37 +870,89 @@ export function Editor() { setNodes((nds) => nds.map((n) => (n.className ? { ...n, className: undefined } : n))); setEdges((eds) => eds.map((e) => (e.className ? { ...e, className: undefined } : e))); setFindings([]); + setThreats([]); flaggedIdsRef.current = new Set(); validationActiveRef.current = false; }, [setNodes, setEdges]); - const runValidate = useCallback(async () => { - let result: Finding[]; + // Analyze the model: generate the STRIDE threat register and, in parallel, the model-hygiene + // findings. Threats (threat-bearing rules) and findings (the rest) are the same detection, so they + // share one panel: the register leads, non-threat findings trail. The register carries the model's + // acceptance triage, so accepted risks come back Accepted. + const runAnalyze = useCallback(async () => { + let generated: Threat[]; + let allFindings: Finding[]; try { - result = await engine.validate(currentModel); + [generated, allFindings] = await Promise.all([ + engine.generateThreats(currentModel), + engine.validate(currentModel), + ]); } catch (err) { - toast(err instanceof Error ? err.message : 'Validation failed.', 'error'); + toast(err instanceof Error ? err.message : 'Analysis failed.', 'error'); return; } - setFindings(result); + setThreats(generated); + // "Other findings" = findings from non-threat-bearing (hygiene) rules; the threat-bearing ones + // are already shown as threats. + const threatRuleIds = new Set(generated.map((t) => t.ruleId)); + setFindings(allFindings.filter((f) => !f.ruleId || !threatRuleIds.has(f.ruleId))); validationActiveRef.current = true; - const flagged = new Set(result.flatMap((f) => f.elementIds)); + const flagged = new Set([...generated.flatMap((t) => t.elementIds), ...allFindings.flatMap((f) => f.elementIds)]); flaggedIdsRef.current = flagged; const applied = applyFlags(nodes, edges, flagged); setNodes(applied.nodes); setEdges(applied.edges); }, [engine, currentModel, nodes, edges, setNodes, setEdges]); - // When a validation is already on screen, changing the rule selection re-runs it so the findings - // reflect the new choice immediately. A ref holds the latest runValidate to avoid a dependency loop. - const runValidateRef = useRef(runValidate); - runValidateRef.current = runValidate; + // When an analysis is already on screen, changing the rule selection re-runs it so the results + // reflect the new choice immediately. A ref holds the latest runAnalyze to avoid a dependency loop. + const runAnalyzeRef = useRef(runAnalyze); + runAnalyzeRef.current = runAnalyze; useEffect(() => { if (validationActiveRef.current) { - void runValidateRef.current(); + void runAnalyzeRef.current(); } }, [disabledRulePacks, disabledRuleIds]); + // Accept a threat's risk with a justification: record it in the model's triage overlay (so it + // persists and round-trips) and reflect it immediately in the panel, no re-analysis needed. + const acceptThreat = useCallback((threat: Threat, reason: string) => { + setThreatTriage((prev) => [...prev.filter((t) => t.id !== threat.id), { id: threat.id, state: 'Accepted', justification: reason }]); + setThreats((prev) => prev.map((t) => (t.id === threat.id ? { ...t, state: 'Accepted', justification: reason } : t))); + }, []); + + // Revert an accepted threat back to open. + const undoAccept = useCallback((threat: Threat) => { + setThreatTriage((prev) => prev.filter((t) => t.id !== threat.id)); + setThreats((prev) => prev.map((t) => (t.id === threat.id ? { ...t, state: 'Open', justification: undefined } : t))); + }, []); + + // Navigates to the page holding the first of a threat's / finding's referenced elements. + const jumpToElements = useCallback( + (elementIds: string[]) => { + for (const id of elementIds) { + const pageId = elementPageIndex.get(id); + if (pageId && pageId !== activePageId) { + switchPage(pageId); + return; + } + } + }, + [elementPageIndex, activePageId, switchPage], + ); + + // The name of the page a threat's elements live on, when that is not the page in view (for a badge). + const offPageLabel = useCallback( + (elementIds: string[]): string | undefined => { + const pageId = elementIds.map((id) => elementPageIndex.get(id)).find(Boolean); + if (!pageId || pageId === activePageId) { + return undefined; + } + return pages.find((p) => p.id === pageId)?.name; + }, + [elementPageIndex, activePageId, pages], + ); + const exportAs = useCallback( async (formatId: string) => { const format = formats.find((f) => f.id === formatId); @@ -935,6 +979,8 @@ export function Editor() { setDisabledRulePacks(nextPacks); setDisabledRuleIds(nextRuleIds); setFindings([]); + setThreats([]); + setThreatTriage(model.threats ?? []); flaggedIdsRef.current = new Set(); validationActiveRef.current = false; setSelection({ node: null, edge: null }); @@ -1008,6 +1054,8 @@ export function Editor() { setNodes([]); setEdges([]); setFindings([]); + setThreats([]); + setThreatTriage([]); flaggedIdsRef.current = new Set(); validationActiveRef.current = false; setSelection({ node: null, edge: null }); @@ -1036,7 +1084,7 @@ export function Editor() { onMerge={() => setShowMerge(true)} dirty={dirty} fileName={fileName} - onValidate={runValidate} + onAnalyze={runAnalyze} onClear={clearAll} onFit={() => fitView({ padding: 0.25, maxZoom: 1.15, duration: 300 })} onUndo={undo} @@ -1069,7 +1117,7 @@ export function Editor() { onNodesDelete={takeSnapshot} onEdgesDelete={takeSnapshot} onSelectionChange={onSelectionChange} - onPaneClick={findings.length ? clearFlags : undefined} + onPaneClick={findings.length || threats.length ? clearFlags : undefined} nodeTypes={nodeTypes} edgeTypes={edgeTypes} defaultEdgeOptions={defaultEdgeOptions} @@ -1120,39 +1168,15 @@ export function Editor() { onToggleRule={toggleRule} /> )} - {findings.length > 0 && ( -
-

- {findings.length} finding{findings.length === 1 ? '' : 's'} -

- {findings.map((f) => { - const pageId = f.elementIds.map((id) => elementPageIndex.get(id)).find(Boolean); - const pageName = - pageId && pageId !== activePageId ? pages.find((p) => p.id === pageId)?.name : undefined; - return ( -
jumpToFinding(f)} - onKeyDown={(event) => { - if (event.key === 'Enter' || event.key === ' ') { - event.preventDefault(); - jumpToFinding(f); - } - }} - > - {f.severity} - - {f.ruleId ? {f.ruleId} : null} {f.message} - {pageName ? {pageName} : null} - -
- ); - })} -
+ {(threats.length > 0 || findings.length > 0) && ( + )} diff --git a/src/ThreatModelForge.Studio/src/dfd/ThreatsPanel.test.tsx b/src/ThreatModelForge.Studio/src/dfd/ThreatsPanel.test.tsx new file mode 100644 index 0000000..c85aa43 --- /dev/null +++ b/src/ThreatModelForge.Studio/src/dfd/ThreatsPanel.test.tsx @@ -0,0 +1,166 @@ +import { describe, it, expect, vi } from 'vitest'; +import { render, screen, fireEvent } from '@testing-library/react'; +import { ThreatsPanel, type ThreatsPanelProps } from './ThreatsPanel'; +import type { Finding, Threat } from './engineClient'; + +function threat(overrides: Partial = {}): Threat { + return { + id: 'element:TM1000', + ruleId: 'TM1000', + category: 'Spoofing', + title: 'A representative threat.', + mitigation: 'Do the mitigating thing.', + severity: 'warning', + priority: 'Medium', + references: [], + elementIds: ['e1'], + interaction: 'Client', + state: 'Open', + ...overrides, + }; +} + +const THREATS: Threat[] = [ + threat({ + id: 'client:TM1023', + ruleId: 'TM1023', + category: 'Spoofing', + title: 'External entity does not authenticate itself.', + references: ['CWE-287', 'CAPEC-151'], + elementIds: ['client'], + interaction: 'Client', + }), + threat({ + id: 'edge1:TM1013', + ruleId: 'TM1013', + category: 'InformationDisclosure', + title: 'Edge lacks a data classification tag.', + references: ['CWE-200'], + elementIds: ['edge1'], + interaction: 'Client -> Gateway [HTTPS]', + }), + threat({ + id: 'edge2:TM1013', + ruleId: 'TM1013', + category: 'InformationDisclosure', + title: 'Another edge lacks a data classification tag.', + references: [], + elementIds: ['edge2'], + interaction: 'Gateway -> Auth [HTTPS]', + }), +]; + +/** Renders the panel with sensible defaults; pass overrides for the props a test cares about. */ +function renderPanel(overrides: Partial = {}): ThreatsPanelProps { + const props: ThreatsPanelProps = { + threats: THREATS, + findings: [], + onSelect: vi.fn(), + offPageLabel: () => undefined, + onAccept: vi.fn(), + onUndoAccept: vi.fn(), + ...overrides, + }; + render(); + return props; +} + +describe('ThreatsPanel', () => { + it('groups threats by STRIDE category, in canonical order, with per-group counts', () => { + renderPanel(); + + expect(screen.getByText('3 open threats')).toBeInTheDocument(); + + const heads = [...document.querySelectorAll('.threat-group-head')].map((h) => h.textContent); + // Spoofing (1) precedes Information disclosure (2) — the canonical STRIDE order, not input order. + expect(heads).toEqual(['Spoofing1', 'Information disclosure2']); + }); + + it('links CWE / CAPEC references to their MITRE catalog pages', () => { + renderPanel(); + + expect(screen.getByRole('link', { name: 'CWE-287' })).toHaveAttribute( + 'href', + 'https://cwe.mitre.org/data/definitions/287.html', + ); + expect(screen.getByRole('link', { name: 'CAPEC-151' })).toHaveAttribute( + 'href', + 'https://capec.mitre.org/data/definitions/151.html', + ); + }); + + it('renders the rule id, interaction scope, and singular "1 open threat" heading', () => { + renderPanel({ threats: [THREATS[0]] }); + + expect(screen.getByText('1 open threat')).toBeInTheDocument(); + expect(screen.getByText('TM1023')).toBeInTheDocument(); + expect(screen.getByText('Client')).toBeInTheDocument(); + }); + + it('shows an off-page badge when a threat lives on another page', () => { + renderPanel({ threats: [THREATS[1]], offPageLabel: () => 'Page 2' }); + + expect(screen.getByText('Page 2')).toBeInTheDocument(); + }); + + it("calls onSelect with the clicked threat's element ids", () => { + const { onSelect } = renderPanel(); + + fireEvent.click(screen.getByText('External entity does not authenticate itself.')); + + expect(onSelect).toHaveBeenCalledTimes(1); + expect(onSelect).toHaveBeenCalledWith(THREATS[0].elementIds); + }); + + it('does not select the threat when a reference link is clicked (stops propagation)', () => { + const { onSelect } = renderPanel(); + + fireEvent.click(screen.getByRole('link', { name: 'CWE-287' })); + + expect(onSelect).not.toHaveBeenCalled(); + }); + + it('accepts a threat with a required justification', () => { + const { onAccept } = renderPanel({ threats: [THREATS[0]] }); + + fireEvent.click(screen.getByRole('button', { name: /^Accept/ })); + const input = screen.getByPlaceholderText('Why is this risk accepted? (required)'); + // The confirm button stays disabled until a justification is entered. + expect(screen.getByRole('button', { name: 'Accept risk' })).toBeDisabled(); + + fireEvent.change(input, { target: { value: 'Compensating control X is in place.' } }); + fireEvent.click(screen.getByRole('button', { name: 'Accept risk' })); + + expect(onAccept).toHaveBeenCalledWith(THREATS[0], 'Compensating control X is in place.'); + }); + + it('shows accepted state (badge + justification) and can undo', () => { + const accepted = threat({ + id: 'client:TM1023', + ruleId: 'TM1023', + title: 'External entity does not authenticate itself.', + state: 'Accepted', + justification: 'Accepted for the pilot.', + elementIds: ['client'], + }); + const { onUndoAccept } = renderPanel({ threats: [accepted] }); + + expect(screen.getByText(/0 open threats/)).toBeInTheDocument(); + expect(screen.getByText(/1 accepted/)).toBeInTheDocument(); + expect(screen.getByText('Accepted')).toBeInTheDocument(); + expect(screen.getByText('“Accepted for the pilot.”')).toBeInTheDocument(); + + fireEvent.click(screen.getByRole('button', { name: 'Undo' })); + expect(onUndoAccept).toHaveBeenCalledWith(accepted); + }); + + it('renders non-threat hygiene findings in an "Other findings" section', () => { + const findings: Finding[] = [ + { id: 'TM1003:0', severity: 'warning', ruleId: 'TM1003', message: 'The model has too few components.', elementIds: [] }, + ]; + renderPanel({ threats: [], findings }); + + expect(screen.getByText('Other findings')).toBeInTheDocument(); + expect(screen.getByText('The model has too few components.')).toBeInTheDocument(); + }); +}); diff --git a/src/ThreatModelForge.Studio/src/dfd/ThreatsPanel.tsx b/src/ThreatModelForge.Studio/src/dfd/ThreatsPanel.tsx new file mode 100644 index 0000000..726d4e8 --- /dev/null +++ b/src/ThreatModelForge.Studio/src/dfd/ThreatsPanel.tsx @@ -0,0 +1,252 @@ +import { useState } from 'react'; +import type { Finding, Threat } from './engineClient'; + +/** STRIDE categories in canonical display order (matches the engine's `StrideCategory`). */ +const STRIDE_ORDER: readonly string[] = [ + 'Spoofing', + 'Tampering', + 'Repudiation', + 'InformationDisclosure', + 'DenialOfService', + 'ElevationOfPrivilege', +]; + +/** Human-readable STRIDE headings. */ +const STRIDE_LABEL: Record = { + Spoofing: 'Spoofing', + Tampering: 'Tampering', + Repudiation: 'Repudiation', + InformationDisclosure: 'Information disclosure', + DenialOfService: 'Denial of service', + ElevationOfPrivilege: 'Elevation of privilege', +}; + +/** Builds a catalog deep-link for a CWE / CAPEC / ATT&CK reference id, or undefined when unrecognized. */ +function referenceUrl(id: string): string | undefined { + const cwe = /^CWE-(\d+)$/i.exec(id); + if (cwe) { + return `https://cwe.mitre.org/data/definitions/${cwe[1]}.html`; + } + const capec = /^CAPEC-(\d+)$/i.exec(id); + if (capec) { + return `https://capec.mitre.org/data/definitions/${capec[1]}.html`; + } + const attack = /^(T\d{4})(?:\.(\d{3}))?$/i.exec(id); + if (attack) { + return `https://attack.mitre.org/techniques/${attack[1]}${attack[2] ? `/${attack[2]}` : ''}/`; + } + return undefined; +} + +export interface ThreatsPanelProps { + /** The generated STRIDE threat register for the current model (open + accepted). */ + threats: Threat[]; + /** Non-threat hygiene findings (structural / naming rules), shown in a trailing section. */ + findings: Finding[]; + /** Navigates to (and reveals) the elements a threat or finding refers to. */ + onSelect: (elementIds: string[]) => void; + /** Returns the name of the page an item's elements live on, when it is not the active page. */ + offPageLabel: (elementIds: string[]) => string | undefined; + /** Accepts a threat's risk with a justification (moves it to the Accepted state). */ + onAccept: (threat: Threat, justification: string) => void; + /** Reverts an accepted threat back to open. */ + onUndoAccept: (threat: Threat) => void; +} + +/** + * The Studio analysis panel: one place for the model's analysis. It leads with the STRIDE threat + * register (grouped by category, each threat acceptable inline with a justification) and trails with + * any non-threat hygiene findings. Threats and findings are the same detection — a threat is a + * threat-bearing finding with a lifecycle — so they share one box rather than two near-identical ones. + */ +export function ThreatsPanel({ threats, findings, onSelect, offPageLabel, onAccept, onUndoAccept }: ThreatsPanelProps) { + const [acceptingId, setAcceptingId] = useState(null); + const [justification, setJustification] = useState(''); + + const byCategory = new Map(); + for (const threat of threats) { + const list = byCategory.get(threat.category) ?? []; + list.push(threat); + byCategory.set(threat.category, list); + } + // Known STRIDE categories first, in canonical order; any unknown category trails, alphabetically. + const known = STRIDE_ORDER.filter((category) => byCategory.has(category)); + const unknown = [...byCategory.keys()].filter((category) => !STRIDE_ORDER.includes(category)).sort(); + const ordered = [...known, ...unknown]; + + const acceptedCount = threats.filter((threat) => threat.state === 'Accepted').length; + const openCount = threats.length - acceptedCount; + + function confirmAccept(threat: Threat) { + const reason = justification.trim(); + if (!reason) { + return; + } + onAccept(threat, reason); + setAcceptingId(null); + setJustification(''); + } + + return ( +
+

+ {openCount} open threat{openCount === 1 ? '' : 's'} + {acceptedCount > 0 ? · {acceptedCount} accepted : null} +

+ {ordered.map((category) => { + const items = byCategory.get(category) ?? []; + return ( +
+
+ {STRIDE_LABEL[category] ?? category} + {items.length} +
+ {items.map((threat) => { + const offPage = offPageLabel(threat.elementIds); + const accepted = threat.state === 'Accepted'; + const isAccepting = acceptingId === threat.id; + return ( +
+
+
onSelect(threat.elementIds)} + onKeyDown={(event) => { + if (event.key === 'Enter' || event.key === ' ') { + event.preventDefault(); + onSelect(threat.elementIds); + } + }} + > + {threat.severity} +
+
+ {threat.ruleId ? {threat.ruleId} : null} {threat.title} + {accepted ? Accepted : null} + {offPage ? {offPage} : null} +
+
+ {threat.interaction ? {threat.interaction} : null} + {threat.references.map((reference) => { + const url = referenceUrl(reference); + return url ? ( + event.stopPropagation()} + > + {reference} + + ) : ( + + {reference} + + ); + })} +
+ {accepted && threat.justification ? ( +
“{threat.justification}”
+ ) : null} +
+
+
+ {accepted ? ( + + ) : !isAccepting ? ( + + ) : null} +
+
+ {isAccepting ? ( +
{ + event.preventDefault(); + confirmAccept(threat); + }} + > +