diff --git a/docs/analysis-rules.md b/docs/analysis-rules.md index b170e55..e5cbc16 100644 --- a/docs/analysis-rules.md +++ b/docs/analysis-rules.md @@ -163,6 +163,86 @@ formats (for example `.tm7`) use the full rule set unless you pass an explicit ` tmforge analyze model.tm7 --ruleset ./my-ruleset.xml ``` +### Authoring custom rules (declarative) + +Ship your own rules as **data**, not code. A declarative rule spec is a JSON file (`*.tmrules.json`) +loaded with `--rules` on [`analyze`](cli-reference.md#analyze), [`threats`](cli-reference.md#threats), +and [`properties`](cli-reference.md#properties): + +```bash +tmforge analyze model.tm7 --rules ./rules.tmrules.json # one spec file +tmforge analyze model.tm7 --rules ./rules/ # a directory of specs (searched recursively) +``` + +Because a spec is inspectable data — not an assembly — it is safe to share and review, and it runs +everywhere the CLI does. Custom rules are **added to** the built-in rules, never a replacement for +them: `--rules` loads your rules *alongside* the full built-in set and both are evaluated together +(custom rules show up in findings, SARIF, `analyze --json`, and — when they read a property — in +`properties --explain`). To narrow the built-ins, use the existing controls independently of +`--rules`: a model's embedded `disabledPacks`, an `--ruleset` override, or `--max-severity`. + +A spec is a `rules` array. A finding is raised for each element of `appliesTo` that matches `when` +(the guard) and fails `assert` (the requirement); at least one of `when`/`assert` is required: + +```jsonc +{ + "rules": [ + { + "id": "ACME001", // your own id namespace (built-ins use TM####) + "pack": "acme-governance", + "severity": "error", // error | warning | info (default: warning) + "appliesTo": "datastore", // process | datastore | external | flow + "message": "Data store {name} does not declare encryption at rest.", + "fullDescription": "Persisted data must be encrypted at rest.", + "helpText": "Set Encrypted to At-rest, TDE, Client-side, or Platform.", + "assert": { "property": "Encrypted", "notAnyOf": ["No"] } + }, + { + "id": "ACME002", + "pack": "acme-governance", + "severity": "warning", + "appliesTo": "flow", + "message": "Flow {name} carries sensitive data in the clear across a trust boundary.", + "stride": "InformationDisclosure", // optional; makes it a `threats` threat + "threatReferences": ["CWE:319"], // optional: CWE: | CAPEC: | ATTACK: + "when": { "property": "DataType", "anyOf": ["EUII", "Customer Content"], "crossesTrustBoundary": true }, + "assert": { "property": "Protocol", "anyOf": ["HTTPS", "TLS", "mTLS"] } + } + ] +} +``` + +The `{name}` token in `message` is replaced with the element's display text. **Conditions** (`when` +and `assert`) are facets that must *all* hold; a bare `property` with no value matcher means "must be +present": + +| Facet | Applies to | Meaning | +| --- | --- | --- | +| `property` + `anyOf` | any | The value is one of the listed values. | +| `property` + `notAnyOf` | any | The value is none of the listed values. | +| `property` + `equals` | any | The value equals a single value. | +| `property` + `present` | any | The property is present (`true`) or absent (`false`). | +| `crossesTrustBoundary` | `flow` | The flow crosses (`true`) or does not cross (`false`) a trust boundary. | +| `source` / `target` | `flow` | A condition on the flow's endpoint: its `kind` (`process`/`datastore`/`external`) and/or a property matcher. | + +- **Ids are your namespace.** A custom rule whose id collides with an already-loaded rule is dropped + with a warning, so the built-in `TM####` namespace always wins (ids appear in SARIF and suppressions). +- **Property names are validated** against the typed [property schema](cli-reference.md#properties). + An unknown property is a warning, not an error — but it catches a typo (`Encryption` vs `Encrypted`) + that would otherwise make a rule silently never match. +- **Resilient loading.** A malformed spec file or an individual invalid rule is reported to standard + error and skipped; the rest still load. +- **Threats.** A custom rule that declares a `stride` category is projected into + [`threats`](cli-reference.md#threats) exactly like a built-in threat-bearing rule. +- **CLI only (and why).** `--rules` works on the CLI; the HTTP API (`/v1`) and the in-browser + (WebAssembly) engine load the built-in rules only. This is deliberate, not an oversight: (1) those + hosts share a **stateless** engine facade — a model in, findings out — with no per-request channel + for selecting rule sources; (2) the **WebAssembly host has no filesystem**, so the file/directory + loader behind `--rules` cannot read spec files there; and (3) loading rules over a shared service is + a security-sensitive contract change (in-memory rule injection, and treating rule-loading as a + privileged action) deferred to a later increment. The rule engine itself is portable the + limitation is the injection surface, not the DSL. + ### Rule variables Some rules read variables supplied on the command line (repeatable): diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 4cb046c..0d05aa2 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -125,7 +125,7 @@ serves at `GET /v1/property-schema`; use it to discover closed enums (for exampl `Encrypted`, `AccessControl`, or the approved cipher list) without running `analyze` first. ```text -tmforge properties [--base ] [--explain] [--json] +tmforge properties [--base ] [--explain] [--rules ] [--json] ``` ```bash @@ -136,7 +136,8 @@ tmforge properties --base datastore --json | jq '.data.properties' Add `--explain` to map each property **value** to the rule id and severity it triggers, so you can predict analysis behavior before running [`analyze`](#analyze). A value shown as `(unset/condition)` means the -rule fires when the property is absent or by a computed condition. +rule fires when the property is absent or by a computed condition. Pass `--rules ` to fold your +[custom declarative rules](analysis-rules.md#authoring-custom-rules-declarative) into the explanation. ```bash tmforge properties --base flow --explain @@ -465,12 +466,13 @@ Evaluate the analysis rules against the model. See [Analysis rules & CI](analysi full rule catalog, packs, and suppressions. ```text -tmforge analyze [--ruleset ] [--suppressionFile ] [--reportFolder ] [--define name=value ...] [--max-severity ] [--json] +tmforge analyze [--ruleset ] [--rules ] [--suppressionFile ] [--reportFolder ] [--define name=value ...] [--max-severity ] [--json] ``` | Option | Meaning | | --- | --- | | `--ruleset ` | Use a custom rule set instead of the built-in default. | +| `--rules ` | Load custom [declarative rules](analysis-rules.md#authoring-custom-rules-declarative) from a `*.tmrules.json` file (or a directory of them) in addition to the built-in rules. | | `--suppressionFile ` | Apply a suppression document to filter findings. | | `--reportFolder ` | Also write SARIF + HTML findings reports (and a JSON listing) to ``. | | `--define name=value` | Repeatable. Supplies a rule variable. | @@ -507,12 +509,13 @@ persisted and triaged (`open -> mitigated -> accepted`). With `--write`, the thr the model's register, keyed so a re-run updates in place and never overwrites prior triage. ```text -tmforge threats [--write] [--json] +tmforge threats [--write] [--rules ] [--json] ``` | Option | Meaning | | --- | --- | | `--write` | Persist the threats into the model's register (preserves prior triage). | +| `--rules ` | Also project threats from custom [declarative rules](analysis-rules.md#authoring-custom-rules-declarative); a custom rule that declares a `stride` category becomes a threat. | 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 diff --git a/src/ThreatModelForge.Analysis/AnalysisRuleSources.cs b/src/ThreatModelForge.Analysis/AnalysisRuleSources.cs new file mode 100644 index 0000000..6e4357d --- /dev/null +++ b/src/ThreatModelForge.Analysis/AnalysisRuleSources.cs @@ -0,0 +1,67 @@ +namespace ThreatModelForge.Analysis +{ + using System; + using System.Collections.Generic; + using System.Reflection; + + /// + /// The single place that assembles a from every rule source: the built-in + /// first-party rules (always) plus any opt-in declarative rules. Every load site — the CLI, the HTTP + /// API, the in-browser engine, threat generation, and the property policy — resolves rules through + /// here, so a custom rule is seen everywhere or nowhere, and there is exactly one place that names + /// the built-in rules assembly. + /// + public static class AnalysisRuleSources + { + private const string BuiltInRulesAssembly = "ThreatModelForge.Analysis.Rules"; + + /// + /// Gets the built-in first-party rule assemblies. This is the only place the built-in rules + /// assembly is named; every load site resolves it here. + /// + /// The built-in rule assemblies. + public static IReadOnlyList BuiltInAssemblies() + { + return new[] { Assembly.Load(BuiltInRulesAssembly) }; + } + + /// + /// Creates a rule set containing the built-in rules plus any declarative rules selected by + /// . A custom rule whose id collides with an already-loaded rule is + /// reported through the options' diagnostics sink and dropped, so the built-in TM#### + /// namespace always wins and ids stay unique across SARIF, suppressions, and JSON output. + /// + /// The opt-in rule sources, or for built-in rules only. + /// A new rule set. The caller owns and disposes it. + public static RuleSet Create(RuleSourceOptions? options = null) + { + Action? diagnostics = options?.Diagnostics; + RuleSet ruleSet = RuleSet.LoadDefault(BuiltInAssemblies(), diagnostics); + + if (options == null || options.SpecPaths.Count == 0) + { + return ruleSet; + } + + HashSet ids = new HashSet(StringComparer.OrdinalIgnoreCase); + foreach (Rule existing in ruleSet.Rules) + { + ids.Add(existing.ID); + } + + foreach (Rule rule in DeclarativeRuleProvider.Load(options.SpecPaths, diagnostics)) + { + if (!ids.Add(rule.ID)) + { + diagnostics?.Invoke($"Skipped custom rule '{rule.ID}': a rule with that id is already loaded."); + rule.Dispose(); + continue; + } + + ruleSet.Rules.Add(rule); + } + + return ruleSet; + } + } +} diff --git a/src/ThreatModelForge.Analysis/DeclarativeCondition.cs b/src/ThreatModelForge.Analysis/DeclarativeCondition.cs new file mode 100644 index 0000000..8c3c7b8 --- /dev/null +++ b/src/ThreatModelForge.Analysis/DeclarativeCondition.cs @@ -0,0 +1,39 @@ +namespace ThreatModelForge.Analysis +{ + using System.Collections.Generic; + using System.Text.Json.Serialization; + + /// + /// A condition over an element. Every specified facet must hold (logical AND). A bare + /// with no value matcher is treated as "must be present". The relational + /// facets — , , and — are + /// only valid on a rule whose appliesTo is flow. + /// + internal sealed class DeclarativeCondition + { + /// Gets or sets the custom-property name the condition reads on the element. + public string? Property { get; set; } + + /// Gets or sets the values that satisfy the condition; the property must equal one of them. + public List? AnyOf { get; set; } + + /// Gets or sets the values that violate the condition; the property must equal none of them. + public List? NotAnyOf { get; set; } + + /// Gets or sets the single value the property must equal. + [JsonPropertyName("equals")] + public string? EqualTo { get; set; } + + /// Gets or sets whether the property must be present (true) or absent (false). + public bool? Present { get; set; } + + /// Gets or sets whether the flow must cross a trust boundary (true) or not (false). + public bool? CrossesTrustBoundary { get; set; } + + /// Gets or sets a condition on the element at the flow's source end. + public DeclarativeEndpoint? Source { get; set; } + + /// Gets or sets a condition on the element at the flow's target end. + public DeclarativeEndpoint? Target { get; set; } + } +} diff --git a/src/ThreatModelForge.Analysis/DeclarativeEndpoint.cs b/src/ThreatModelForge.Analysis/DeclarativeEndpoint.cs new file mode 100644 index 0000000..9e8eea0 --- /dev/null +++ b/src/ThreatModelForge.Analysis/DeclarativeEndpoint.cs @@ -0,0 +1,31 @@ +namespace ThreatModelForge.Analysis +{ + using System.Collections.Generic; + using System.Text.Json.Serialization; + + /// + /// A condition on the element at one end of a flow, resolved through the flow's source or target + /// reference. Every specified facet must hold (logical AND). + /// + internal sealed class DeclarativeEndpoint + { + /// Gets or sets the required kind of the endpoint element (process, datastore, or external). + public string? Kind { get; set; } + + /// Gets or sets the custom-property name to read on the endpoint element. + public string? Property { get; set; } + + /// Gets or sets the values that satisfy the condition; the property must equal one of them. + public List? AnyOf { get; set; } + + /// Gets or sets the values that violate the condition; the property must equal none of them. + public List? NotAnyOf { get; set; } + + /// Gets or sets the single value the property must equal. + [JsonPropertyName("equals")] + public string? EqualTo { get; set; } + + /// Gets or sets whether the property must be present (true) or absent (false). + public bool? Present { get; set; } + } +} diff --git a/src/ThreatModelForge.Analysis/DeclarativeRule.cs b/src/ThreatModelForge.Analysis/DeclarativeRule.cs new file mode 100644 index 0000000..d73cbfb --- /dev/null +++ b/src/ThreatModelForge.Analysis/DeclarativeRule.cs @@ -0,0 +1,274 @@ +namespace ThreatModelForge.Analysis +{ + using System; + using System.Collections.Generic; + using System.Linq; + using ThreatModelForge.Model; + using ThreatModelForge.Model.Abstracts; + + /// + /// A whose behavior is defined by data (a ) + /// rather than compiled code. It evaluates a guard/requirement pair over the elements of a single + /// DFD primitive, reusing the same model helpers () the built-in rules use, + /// so a declarative rule resolves trust-boundary crossings and endpoint kinds identically to a + /// first-party rule. A finding is raised for each candidate that matches the guard and fails the + /// requirement. + /// + internal sealed class DeclarativeRule : Rule + { + private readonly string appliesTo; + private readonly string messageTemplate; + private readonly StrideCategory? stride; + private readonly IReadOnlyList threatReferences; + private readonly IReadOnlyList propertyBindings; + private readonly DeclarativeCondition? whenCondition; + private readonly DeclarativeCondition? assertCondition; + + /// + /// Initializes a new instance of the class. + /// + /// The rule id. + /// The rule pack id. + /// The rule severity. + /// The DFD primitive the rule targets. + /// The finding message template ({name} is substituted). + /// The long description. + /// The remediation guidance. + /// The optional documentation URL. + /// The optional STRIDE category. + /// The external references. + /// The optional guard condition. + /// The optional requirement condition. + internal DeclarativeRule( + string id, + string pack, + MessageSeverity severity, + string appliesTo, + string messageTemplate, + string fullDescription, + string helpText, + Uri? helpUri, + StrideCategory? stride, + IReadOnlyList threatReferences, + DeclarativeCondition? when, + DeclarativeCondition? assert) + : base(id, severity, pack) + { + this.appliesTo = appliesTo; + this.messageTemplate = messageTemplate; + this.FullDescription = fullDescription; + this.HelpText = helpText; + this.HelpUri = helpUri; + this.stride = stride; + this.threatReferences = threatReferences; + this.whenCondition = when; + this.assertCondition = assert; + this.propertyBindings = BuildBindings(appliesTo, when, assert); + } + + /// + public override StrideCategory? Stride => this.stride; + + /// + public override IReadOnlyList ThreatReferences => this.threatReferences; + + /// + public override IReadOnlyList PropertyBindings => this.propertyBindings; + + /// + public override void Evaluate(RuleEvaluationContext context) + { + _ = context ?? throw new ArgumentNullException(nameof(context)); + + foreach (DrawingSurfaceModel diagram in context.Model.DrawingSurfaceList) + { + foreach (Entity element in this.Candidates(diagram)) + { + if (this.whenCondition != null && !EvaluateCondition(this.whenCondition, diagram, element)) + { + continue; + } + + // A finding is raised when the guard matches and the requirement is not satisfied. + // A rule with no requirement flags every guard match unconditionally. + if (this.assertCondition != null && EvaluateCondition(this.assertCondition, diagram, element)) + { + continue; + } + + string text = this.messageTemplate.Replace("{name}", GetEntityDisplayText(element)); + context.Writer.Write(this.CreateMessage(element, diagram, text)); + } + } + } + + private static bool MatchesKind(Entity entity, string kind) + { + if (string.Equals(kind, "external", StringComparison.OrdinalIgnoreCase)) + { + return entity.IsExternalInteractor(); + } + + if (string.Equals(kind, "datastore", StringComparison.OrdinalIgnoreCase)) + { + return entity.IsStorageComponent(); + } + + if (string.Equals(kind, "process", StringComparison.OrdinalIgnoreCase)) + { + return entity.IsComponent() && !entity.IsExternalInteractor() && !entity.IsStorageComponent(); + } + + return false; + } + + private static bool EvaluateCondition(DeclarativeCondition condition, DrawingSurfaceModel diagram, Entity element) + { + if (condition.Property != null && + !MatchProperty(element, condition.Property, condition.AnyOf, condition.NotAnyOf, condition.EqualTo, condition.Present)) + { + return false; + } + + if (condition.CrossesTrustBoundary.HasValue) + { + bool crosses = element is Connector connector && diagram.TrustBoundaryCrossings(connector).Any(); + if (crosses != condition.CrossesTrustBoundary.Value) + { + return false; + } + } + + if (condition.Source != null && !MatchEndpoint(diagram, element, condition.Source, isSource: true)) + { + return false; + } + + if (condition.Target != null && !MatchEndpoint(diagram, element, condition.Target, isSource: false)) + { + return false; + } + + return true; + } + + private static bool MatchEndpoint(DrawingSurfaceModel diagram, Entity element, DeclarativeEndpoint endpoint, bool isSource) + { + if (element is not Connector connector) + { + return false; + } + + Guid guid = isSource ? connector.SourceGuid : connector.TargetGuid; + if (!diagram.Borders.TryGetValue(guid, out object? value) || value is not Entity endpointEntity) + { + return false; + } + + if (endpoint.Kind != null && !MatchesKind(endpointEntity, endpoint.Kind)) + { + return false; + } + + if (endpoint.Property != null && + !MatchProperty(endpointEntity, endpoint.Property, endpoint.AnyOf, endpoint.NotAnyOf, endpoint.EqualTo, endpoint.Present)) + { + return false; + } + + return true; + } + + private static bool MatchProperty( + Entity entity, + string property, + IReadOnlyList? anyOf, + IReadOnlyList? notAnyOf, + string? equalTo, + bool? present) + { + bool defined = entity.TryGetCustomPropertyValue(property, out string? raw); + string value = raw ?? string.Empty; + bool isPresent = defined && !string.IsNullOrWhiteSpace(value); + + bool hasMatcher = present.HasValue || + equalTo != null || + (anyOf != null && anyOf.Count > 0) || + (notAnyOf != null && notAnyOf.Count > 0); + + // A bare property with no value matcher means "the property must be present". + if (!hasMatcher) + { + return isPresent; + } + + if (present.HasValue && isPresent != present.Value) + { + return false; + } + + if (equalTo != null && (!isPresent || !string.Equals(value, equalTo, StringComparison.OrdinalIgnoreCase))) + { + return false; + } + + if (anyOf != null && anyOf.Count > 0 && (!isPresent || !anyOf.Contains(value, StringComparer.OrdinalIgnoreCase))) + { + return false; + } + + if (notAnyOf != null && notAnyOf.Count > 0 && isPresent && notAnyOf.Contains(value, StringComparer.OrdinalIgnoreCase)) + { + return false; + } + + return true; + } + + private static IReadOnlyList BuildBindings(string appliesTo, DeclarativeCondition? when, DeclarativeCondition? assert) + { + List bindings = new List(); + AddBinding(bindings, appliesTo, when); + AddBinding(bindings, appliesTo, assert); + return bindings; + } + + private static void AddBinding(List bindings, string appliesTo, DeclarativeCondition? condition) + { + if (condition == null) + { + return; + } + + if (condition.Property != null) + { + string[] flagged = (condition.NotAnyOf ?? new List()).ToArray(); + bindings.Add(new PropertyBinding(appliesTo, condition.Property, flagged)); + } + + AddEndpointBinding(bindings, condition.Source); + AddEndpointBinding(bindings, condition.Target); + } + + private static void AddEndpointBinding(List bindings, DeclarativeEndpoint? endpoint) + { + if (endpoint?.Property == null || endpoint.Kind == null) + { + return; + } + + string[] flagged = (endpoint.NotAnyOf ?? new List()).ToArray(); + bindings.Add(new PropertyBinding(endpoint.Kind, endpoint.Property, flagged)); + } + + private IEnumerable Candidates(DrawingSurfaceModel diagram) + { + if (string.Equals(this.appliesTo, "flow", StringComparison.OrdinalIgnoreCase)) + { + return diagram.Lines.Values.OfType(); + } + + return diagram.Components().Where(entity => MatchesKind(entity, this.appliesTo)); + } + } +} diff --git a/src/ThreatModelForge.Analysis/DeclarativeRuleFile.cs b/src/ThreatModelForge.Analysis/DeclarativeRuleFile.cs new file mode 100644 index 0000000..c55cd68 --- /dev/null +++ b/src/ThreatModelForge.Analysis/DeclarativeRuleFile.cs @@ -0,0 +1,16 @@ +namespace ThreatModelForge.Analysis +{ + using System.Collections.Generic; + + /// + /// The root of a declarative rule spec file (*.tmrules.json): a list of rule definitions + /// authored as data rather than compiled code. Because a spec is inspectable data — not arbitrary + /// code — it is the safe way to ship shared or third-party rule packs. Deserialized by + /// . + /// + internal sealed class DeclarativeRuleFile + { + /// Gets or sets the rules declared in the file. + public List? Rules { get; set; } + } +} diff --git a/src/ThreatModelForge.Analysis/DeclarativeRuleProvider.cs b/src/ThreatModelForge.Analysis/DeclarativeRuleProvider.cs new file mode 100644 index 0000000..9112911 --- /dev/null +++ b/src/ThreatModelForge.Analysis/DeclarativeRuleProvider.cs @@ -0,0 +1,323 @@ +namespace ThreatModelForge.Analysis +{ + using System; + using System.Collections.Generic; + using System.Globalization; + using System.IO; + using System.Linq; + using System.Text.Json; + using ThreatModelForge.Editing; + + /// + /// Loads instances from declarative spec files (*.tmrules.json). + /// Loading is resilient: a spec file that cannot be read or parsed, or an individual rule that fails + /// validation, is reported through the diagnostics sink and skipped rather than aborting the load, so + /// one malformed rule never takes down the whole set. Property names are validated against the + /// typed property schema and produce a warning (not a failure) when unknown, because custom + /// properties are open-ended. + /// + public static class DeclarativeRuleProvider + { + private static readonly JsonSerializerOptions Options = new JsonSerializerOptions + { + PropertyNameCaseInsensitive = true, + ReadCommentHandling = JsonCommentHandling.Skip, + AllowTrailingCommas = true, + }; + + private static readonly HashSet Kinds = new HashSet( + new[] { "process", "datastore", "external", "flow" }, + StringComparer.OrdinalIgnoreCase); + + /// + /// Loads all declarative rules from the given spec paths. A path may be a single + /// *.tmrules.json file or a directory that is searched recursively for them. + /// + /// The spec files or directories to load. + /// An optional sink for non-fatal load and validation warnings. + /// The loaded rules, in file then declaration order. + public static IReadOnlyList Load(IEnumerable paths, Action? diagnostics = null) + { + if (paths == null) + { + throw new ArgumentNullException(nameof(paths)); + } + + List rules = new List(); + foreach (string path in paths) + { + foreach (string file in ExpandFiles(path, diagnostics)) + { + LoadFile(file, rules, diagnostics); + } + } + + return rules; + } + + private static IEnumerable ExpandFiles(string path, Action? diagnostics) + { + if (string.IsNullOrWhiteSpace(path)) + { + return Array.Empty(); + } + + if (Directory.Exists(path)) + { + return Directory.EnumerateFiles(path, "*.tmrules.json", SearchOption.AllDirectories) + .OrderBy(p => p, StringComparer.Ordinal) + .ToList(); + } + + if (File.Exists(path)) + { + return new[] { path }; + } + + diagnostics?.Invoke($"Rule source not found: '{path}'."); + return Array.Empty(); + } + + private static void LoadFile(string file, List rules, Action? diagnostics) + { + DeclarativeRuleFile? parsed; + try + { + string json = File.ReadAllText(file); + parsed = JsonSerializer.Deserialize(json, Options); + } + catch (Exception ex) when (ex is IOException || ex is JsonException || ex is UnauthorizedAccessException) + { + diagnostics?.Invoke($"Skipped rule file '{file}': {ex.Message}"); + return; + } + + if (parsed?.Rules == null) + { + return; + } + + foreach (DeclarativeRuleSpec spec in parsed.Rules) + { + Rule? rule = Compile(spec, file, diagnostics); + if (rule != null) + { + rules.Add(rule); + } + } + } + + private static Rule? Compile(DeclarativeRuleSpec spec, string file, Action? diagnostics) + { + string origin = $"rule '{spec.Id ?? "(no id)"}' in '{file}'"; + + if (string.IsNullOrWhiteSpace(spec.Id)) + { + diagnostics?.Invoke($"Skipped a rule in '{file}': missing 'id'."); + return null; + } + + if (string.IsNullOrWhiteSpace(spec.AppliesTo) || !Kinds.Contains(spec.AppliesTo!)) + { + diagnostics?.Invoke($"Skipped {origin}: 'appliesTo' must be one of process, datastore, external, flow."); + return null; + } + + if (string.IsNullOrWhiteSpace(spec.Message)) + { + diagnostics?.Invoke($"Skipped {origin}: missing 'message'."); + return null; + } + + if (spec.When == null && spec.Assert == null) + { + diagnostics?.Invoke($"Skipped {origin}: at least one of 'when' or 'assert' is required."); + return null; + } + + bool isFlow = string.Equals(spec.AppliesTo, "flow", StringComparison.OrdinalIgnoreCase); + if (!isFlow && (UsesRelational(spec.When) || UsesRelational(spec.Assert))) + { + diagnostics?.Invoke($"Skipped {origin}: 'crossesTrustBoundary', 'source', and 'target' require appliesTo 'flow'."); + return null; + } + + if (!TryParseSeverity(spec.Severity, out MessageSeverity severity)) + { + diagnostics?.Invoke($"Skipped {origin}: unknown severity '{spec.Severity}'."); + return null; + } + + if (!TryParseStride(spec.Stride, out StrideCategory? stride)) + { + diagnostics?.Invoke($"Skipped {origin}: unknown stride '{spec.Stride}'."); + return null; + } + + Uri? helpUri = null; + if (!string.IsNullOrWhiteSpace(spec.HelpUri) && !Uri.TryCreate(spec.HelpUri, UriKind.Absolute, out helpUri)) + { + diagnostics?.Invoke($"Ignored invalid 'helpUri' on {origin}: '{spec.HelpUri}'."); + helpUri = null; + } + + List references = ParseReferences(spec.ThreatReferences, origin, diagnostics); + WarnUnknownProperties(spec, origin, diagnostics); + + string message = spec.Message!; + return new DeclarativeRule( + spec.Id!, + string.IsNullOrWhiteSpace(spec.Pack) ? "custom" : spec.Pack!, + severity, + spec.AppliesTo!, + message, + string.IsNullOrWhiteSpace(spec.FullDescription) ? message : spec.FullDescription!, + spec.HelpText ?? string.Empty, + helpUri, + stride, + references, + spec.When, + spec.Assert); + } + + private static bool UsesRelational(DeclarativeCondition? condition) + { + return condition != null && + (condition.CrossesTrustBoundary.HasValue || condition.Source != null || condition.Target != null); + } + + private static bool TryParseSeverity(string? value, out MessageSeverity severity) + { + if (string.IsNullOrWhiteSpace(value)) + { + severity = MessageSeverity.Warning; + return true; + } + + return Enum.TryParse(value, ignoreCase: true, out severity); + } + + private static bool TryParseStride(string? value, out StrideCategory? stride) + { + stride = null; + if (string.IsNullOrWhiteSpace(value)) + { + return true; + } + + if (Enum.TryParse(value, ignoreCase: true, out StrideCategory parsed)) + { + stride = parsed; + return true; + } + + return false; + } + + private static List ParseReferences(List? references, string origin, Action? diagnostics) + { + List result = new List(); + if (references == null) + { + return result; + } + + foreach (string reference in references) + { + ThreatReference? parsed = ParseReference(reference); + if (parsed == null) + { + diagnostics?.Invoke($"Ignored unrecognized threat reference '{reference}' on {origin}."); + continue; + } + + result.Add(parsed); + } + + return result; + } + + private static ThreatReference? ParseReference(string reference) + { + if (string.IsNullOrWhiteSpace(reference)) + { + return null; + } + + int separator = reference.IndexOf(':'); + if (separator <= 0) + { + return null; + } + + string catalog = reference.Substring(0, separator).Trim(); + string id = reference.Substring(separator + 1).Trim(); + if (id.Length == 0) + { + return null; + } + + if (string.Equals(catalog, "CWE", StringComparison.OrdinalIgnoreCase) && + int.TryParse(id, NumberStyles.Integer, CultureInfo.InvariantCulture, out int cwe)) + { + return ThreatReference.Cwe(cwe); + } + + if (string.Equals(catalog, "CAPEC", StringComparison.OrdinalIgnoreCase) && + int.TryParse(id, NumberStyles.Integer, CultureInfo.InvariantCulture, out int capec)) + { + return ThreatReference.Capec(capec); + } + + if (string.Equals(catalog, "ATTACK", StringComparison.OrdinalIgnoreCase)) + { + return ThreatReference.Attack(id); + } + + return null; + } + + private static void WarnUnknownProperties(DeclarativeRuleSpec spec, string origin, Action? diagnostics) + { + if (diagnostics == null) + { + return; + } + + WarnProperty(spec.AppliesTo!, spec.When?.Property, origin, diagnostics); + WarnProperty(spec.AppliesTo!, spec.Assert?.Property, origin, diagnostics); + WarnEndpointProperties(spec.When, origin, diagnostics); + WarnEndpointProperties(spec.Assert, origin, diagnostics); + } + + private static void WarnEndpointProperties(DeclarativeCondition? condition, string origin, Action diagnostics) + { + if (condition?.Source?.Kind != null) + { + WarnProperty(condition.Source.Kind!, condition.Source.Property, origin, diagnostics); + } + + if (condition?.Target?.Kind != null) + { + WarnProperty(condition.Target.Kind!, condition.Target.Property, origin, diagnostics); + } + } + + private static void WarnProperty(string appliesTo, string? property, string origin, Action diagnostics) + { + if (string.IsNullOrWhiteSpace(property)) + { + return; + } + + bool known = PropertySchemaCatalog.All.Any(descriptor => + string.Equals(descriptor.AppliesTo, appliesTo, StringComparison.OrdinalIgnoreCase) && + string.Equals(descriptor.Name, property, StringComparison.OrdinalIgnoreCase)); + + if (!known) + { + diagnostics($"{origin} reads property '{property}' on '{appliesTo}', which is not in the property schema."); + } + } + } +} diff --git a/src/ThreatModelForge.Analysis/DeclarativeRuleSpec.cs b/src/ThreatModelForge.Analysis/DeclarativeRuleSpec.cs new file mode 100644 index 0000000..65162a4 --- /dev/null +++ b/src/ThreatModelForge.Analysis/DeclarativeRuleSpec.cs @@ -0,0 +1,49 @@ +namespace ThreatModelForge.Analysis +{ + using System.Collections.Generic; + + /// + /// A single declarative rule. A finding is raised for each element of that + /// matches (or every such element when is omitted) and fails + /// (or unconditionally when is omitted). At least one of + /// or must be present. + /// + internal sealed class DeclarativeRuleSpec + { + /// Gets or sets the unique rule id in the pack's own namespace (for example ACME001). + public string? Id { get; set; } + + /// Gets or sets the identifier of the rule pack this rule belongs to. + public string? Pack { get; set; } + + /// Gets or sets the severity (error, warning, or info); defaults to warning. + public string? Severity { get; set; } + + /// Gets or sets the DFD primitive the rule targets (process, datastore, external, or flow). + public string? AppliesTo { get; set; } + + /// Gets or sets the finding message; the token {name} is replaced with the element's display text. + public string? Message { get; set; } + + /// Gets or sets the long description of what the rule checks and why. + public string? FullDescription { get; set; } + + /// Gets or sets guidance on how to resolve a finding. + public string? HelpText { get; set; } + + /// Gets or sets an optional documentation URL. + public string? HelpUri { get; set; } + + /// Gets or sets the optional STRIDE category the finding represents (enables threat generation). + public string? Stride { get; set; } + + /// Gets or sets the optional external references (for example CWE:319, CAPEC:157, ATTACK:T1040). + public List? ThreatReferences { get; set; } + + /// Gets or sets the optional guard; the rule only evaluates elements that match it. + public DeclarativeCondition? When { get; set; } + + /// Gets or sets the requirement; a matching element that fails it produces a finding. + public DeclarativeCondition? Assert { get; set; } + } +} diff --git a/src/ThreatModelForge.Analysis/KnowledgeBaseCatalog.cs b/src/ThreatModelForge.Analysis/KnowledgeBaseCatalog.cs index 2dd4560..252f520 100644 --- a/src/ThreatModelForge.Analysis/KnowledgeBaseCatalog.cs +++ b/src/ThreatModelForge.Analysis/KnowledgeBaseCatalog.cs @@ -242,8 +242,7 @@ private static string CategoryLetter(StrideCategory category) private static RuleSet LoadDefaultRuleSet() { - Assembly rules = Assembly.Load("ThreatModelForge.Analysis.Rules"); - return RuleSet.LoadDefault(new[] { rules }); + return AnalysisRuleSources.Create(); } private readonly struct GenericMapping diff --git a/src/ThreatModelForge.Analysis/Rule.cs b/src/ThreatModelForge.Analysis/Rule.cs index 2c5cbc2..1044e21 100644 --- a/src/ThreatModelForge.Analysis/Rule.cs +++ b/src/ThreatModelForge.Analysis/Rule.cs @@ -11,16 +11,36 @@ namespace ThreatModelForge.Analysis /// public abstract class Rule : IDisposable { - private readonly int ruleId; + private readonly string ruleId; /// - /// Initializes a new instance of the class. + /// Initializes a new instance of the class with a built-in numeric id, + /// surfaced as TM<id>. This is the convention for first-party rules. /// - /// The unique id of the rule. + /// The unique numeric id of the rule; surfaced as TM{id}. /// The default severity for the rule. /// The identifier of the rule pack this rule belongs to. protected Rule(int id, MessageSeverity defaultSeverity, string pack) + : this($"TM{id}", defaultSeverity, pack) + { + } + + /// + /// Initializes a new instance of the class with an explicit string id. + /// Third-party rule packs use this to declare their own id namespace (for example + /// ACME001) rather than the built-in TM#### convention, so their ids do not + /// collide with first-party ids in SARIF, suppressions, or JSON output. + /// + /// The unique id of the rule. + /// The default severity for the rule. + /// The identifier of the rule pack this rule belongs to. + protected Rule(string id, MessageSeverity defaultSeverity, string pack) { + if (string.IsNullOrWhiteSpace(id)) + { + throw new ArgumentException("Value cannot be null or whitespace.", nameof(id)); + } + this.ruleId = id; this.Severity = defaultSeverity; this.Pack = pack ?? throw new ArgumentNullException(nameof(pack)); @@ -40,13 +60,7 @@ protected Rule(int id, MessageSeverity defaultSeverity, string pack) /// /// Gets the rule id. /// - public string ID - { - get - { - return $"TM{this.ruleId}"; - } - } + public string ID => this.ruleId; /// /// Gets the identifier of the rule pack this rule belongs to. diff --git a/src/ThreatModelForge.Analysis/RuleSet.cs b/src/ThreatModelForge.Analysis/RuleSet.cs index d4a99d0..2c627d9 100644 --- a/src/ThreatModelForge.Analysis/RuleSet.cs +++ b/src/ThreatModelForge.Analysis/RuleSet.cs @@ -3,6 +3,7 @@ namespace ThreatModelForge.Analysis using System; using System.Collections.Generic; using System.Collections.ObjectModel; + using System.IO; using System.Linq; using System.Reflection; @@ -19,12 +20,18 @@ public class RuleSet : IDisposable public Collection Rules { get; } = new Collection(); /// - /// Loads a default rule set based on the rules defined in the given assemblies. + /// Loads a default rule set based on the rules defined in the given assemblies. Discovery is + /// resilient to third-party input: an assembly whose types cannot be enumerated, an abstract + /// subclass, a rule without a public parameterless constructor, or a rule + /// whose constructor throws is reported through and skipped + /// rather than aborting the whole load. /// /// The analysis assemblies. + /// An optional sink for non-fatal load warnings. /// A new instance of the class. public static RuleSet LoadDefault( - IEnumerable analysisAssemblies) + IEnumerable analysisAssemblies, + Action? diagnostics = null) { if (analysisAssemblies == null) { @@ -34,15 +41,43 @@ public static RuleSet LoadDefault( RuleSet result = new RuleSet(); foreach (Assembly asm in analysisAssemblies) { - if (asm != null) + if (asm == null) + { + continue; + } + + Type[] types; + try + { + types = asm.GetExportedTypes(); + } + catch (Exception ex) when (ex is ReflectionTypeLoadException || ex is FileNotFoundException || ex is FileLoadException || ex is TypeLoadException) + { + diagnostics?.Invoke($"Skipped rule assembly '{asm.GetName().Name}': {ex.Message}"); + continue; + } + + foreach (Type t in types) { - foreach (Type t in asm.GetExportedTypes()) + if (!typeof(Rule).IsAssignableFrom(t) || t.IsAbstract) { - if (typeof(Rule).IsAssignableFrom(t)) - { - Rule r = (Rule)Activator.CreateInstance(t); - result.Rules.Add(r); - } + continue; + } + + if (t.GetConstructor(Type.EmptyTypes) == null) + { + diagnostics?.Invoke($"Skipped rule '{t.FullName}': no public parameterless constructor."); + continue; + } + + try + { + Rule r = (Rule)Activator.CreateInstance(t); + result.Rules.Add(r); + } + catch (Exception ex) when (ex is MissingMethodException || ex is MemberAccessException || ex is TargetInvocationException) + { + diagnostics?.Invoke($"Skipped rule '{t.FullName}': {ex.Message}"); } } } @@ -67,13 +102,26 @@ public static RuleSet Load( IEnumerable analysisAssemblies) { RuleSet result = LoadDefault(analysisAssemblies); + result.ApplyOverrides(path); + return result; + } + + /// + /// Applies the severity and disable overrides from a .ruleset file to the rules in + /// this set. Rule ids listed in the file that are not present in the set are ignored, so the + /// same override file composes over any rule source (built-in or custom). Matching is + /// case-insensitive. + /// + /// The path to the .ruleset file. + public void ApplyOverrides(string path) + { Xml.RuleSetXml xml = Xml.RuleSetXml.Load(path); foreach (Xml.RulesXml ruleBlock in xml.Rules) { foreach (Xml.RuleXml ruleOverride in ruleBlock.Rules) { - Rule rule = result.Rules.FirstOrDefault( + Rule rule = this.Rules.FirstOrDefault( e => string.Equals(e.ID, ruleOverride.Id, StringComparison.OrdinalIgnoreCase)); if (rule == null) { @@ -97,8 +145,6 @@ public static RuleSet Load( } } } - - return result; } /// diff --git a/src/ThreatModelForge.Analysis/RuleSourceOptions.cs b/src/ThreatModelForge.Analysis/RuleSourceOptions.cs new file mode 100644 index 0000000..6402844 --- /dev/null +++ b/src/ThreatModelForge.Analysis/RuleSourceOptions.cs @@ -0,0 +1,31 @@ +namespace ThreatModelForge.Analysis +{ + using System; + using System.Collections.Generic; + using System.Linq; + + /// + /// Options that select which rule sources contribute to a loaded beyond the + /// built-in first-party rules. Custom rules are strictly opt-in: an empty set of options yields only + /// the built-in rules. + /// + public sealed class RuleSourceOptions + { + /// + /// Initializes a new instance of the class. + /// + /// The declarative rule spec files or directories to load, or for none. + /// An optional sink for non-fatal load and validation warnings. + public RuleSourceOptions(IEnumerable? specPaths = null, Action? diagnostics = null) + { + this.SpecPaths = specPaths?.Where(path => !string.IsNullOrWhiteSpace(path)).ToList() ?? new List(); + this.Diagnostics = diagnostics; + } + + /// Gets the declarative rule spec files or directories to load. + public IReadOnlyList SpecPaths { get; } + + /// Gets the optional sink for non-fatal load and validation warnings. + public Action? Diagnostics { get; } + } +} diff --git a/src/ThreatModelForge.Analysis/ThreatGenerator.cs b/src/ThreatModelForge.Analysis/ThreatGenerator.cs index 43561b2..e5713a5 100644 --- a/src/ThreatModelForge.Analysis/ThreatGenerator.cs +++ b/src/ThreatModelForge.Analysis/ThreatGenerator.cs @@ -4,7 +4,6 @@ namespace ThreatModelForge.Analysis using System.Collections.Generic; using System.Globalization; using System.Linq; - using System.Reflection; using ThreatModelForge.KnowledgeBase; using ThreatModelForge.Model; using ThreatModelForge.Model.Abstracts; @@ -165,8 +164,7 @@ public static bool Accept(ThreatModel model, string threatId, string reason) private static RuleSet LoadDefaultRuleSet() { - Assembly rules = Assembly.Load("ThreatModelForge.Analysis.Rules"); - return RuleSet.LoadDefault(new[] { rules }); + return AnalysisRuleSources.Create(); } private static GeneratedThreat ToThreat(Message message, Rule rule, Entity target, StrideCategory category) diff --git a/src/ThreatModelForge.Cli/AnalyzeArguments.cs b/src/ThreatModelForge.Cli/AnalyzeArguments.cs index 78d5ca7..71e4cc6 100644 --- a/src/ThreatModelForge.Cli/AnalyzeArguments.cs +++ b/src/ThreatModelForge.Cli/AnalyzeArguments.cs @@ -16,6 +16,7 @@ internal class AnalyzeArguments "suppressionFile", "reportFolder", "max-severity", + "rules", }; private AnalyzeArguments( @@ -23,6 +24,7 @@ private AnalyzeArguments( string ruleSetPath, string suppressionFilePath, string reportFolderPath, + string rulePath, IReadOnlyDictionary variables, bool json, MessageSeverity maxSeverity) @@ -33,6 +35,7 @@ private AnalyzeArguments( this.RuleSetPath = ruleSetPath; this.SuppressionFilePath = suppressionFilePath; this.ReportFolderPath = reportFolderPath; + this.RulePath = rulePath; this.Json = json; this.MaxSeverity = maxSeverity; if (variables == null) @@ -66,6 +69,11 @@ private AnalyzeArguments( /// public string ReportFolderPath { get; } + /// + /// Gets the optional path to a declarative custom rule spec file or a directory of them. + /// + public string RulePath { get; } + /// /// Gets a value indicating whether machine-readable JSON output was requested. /// @@ -157,6 +165,7 @@ public static bool TryParse(string[] args, out AnalyzeArguments? result) parsed.Get("ruleset") ?? string.Empty, parsed.Get("suppressionFile") ?? string.Empty, parsed.Get("reportFolder") ?? string.Empty, + parsed.Get("rules") ?? string.Empty, variables, parsed.Json, maxSeverity); diff --git a/src/ThreatModelForge.Cli/AnalyzeCommand.cs b/src/ThreatModelForge.Cli/AnalyzeCommand.cs index 61e0f09..06ab532 100644 --- a/src/ThreatModelForge.Cli/AnalyzeCommand.cs +++ b/src/ThreatModelForge.Cli/AnalyzeCommand.cs @@ -42,7 +42,7 @@ public static int Run(string[] args) return ErrorExitCode; } - using (RuleSet ruleSet = GetRuleSet(arguments!.RuleSetPath)) + using (RuleSet ruleSet = GetRuleSet(arguments!.RuleSetPath, arguments.RulePath)) { if (string.IsNullOrWhiteSpace(arguments.RuleSetPath)) { @@ -176,20 +176,15 @@ private static void WriteReports( } } - private static RuleSet GetRuleSet(string ruleSetPath) + private static RuleSet GetRuleSet(string ruleSetPath, string rulePath) { - IEnumerable analysisAssemblies = LoadAnalysisAssemblies(); - return string.IsNullOrWhiteSpace(ruleSetPath) ? - RuleSet.LoadDefault(analysisAssemblies) : - RuleSet.Load(ruleSetPath, analysisAssemblies); - } - - private static IEnumerable LoadAnalysisAssemblies() - { - return new[] + RuleSet ruleSet = AnalysisRuleSources.Create(RuleSourceCli.FromPath(rulePath)); + if (!string.IsNullOrWhiteSpace(ruleSetPath)) { - Assembly.Load("ThreatModelForge.Analysis.Rules"), - }; + ruleSet.ApplyOverrides(ruleSetPath); + } + + return ruleSet; } private static ToolInfo LoadToolInfo() diff --git a/src/ThreatModelForge.Cli/Properties/Resources.resx b/src/ThreatModelForge.Cli/Properties/Resources.resx index 3c90020..cdd0e7a 100644 --- a/src/ThreatModelForge.Cli/Properties/Resources.resx +++ b/src/ThreatModelForge.Cli/Properties/Resources.resx @@ -138,6 +138,7 @@ Options: --json Emit machine-readable JSON to standard output. --define <name>=<value> Define a variable <name> with the given <value>. --ruleset <path> Path to a .ruleset file to use. Optional. + --rules <path> Path to a declarative custom rule spec (*.tmrules.json) or a directory of them. Optional. --suppressionFile <path> Path to a suppression .json file to use. Optional. --reportFolder <path> Path to a folder where report output files will be written. Optional. --max-severity <level> Gate the exit code on findings at or above <level> (error|warning|info). Default: error. diff --git a/src/ThreatModelForge.Cli/PropertiesCommand.cs b/src/ThreatModelForge.Cli/PropertiesCommand.cs index 0e02f86..b5fae8b 100644 --- a/src/ThreatModelForge.Cli/PropertiesCommand.cs +++ b/src/ThreatModelForge.Cli/PropertiesCommand.cs @@ -3,7 +3,7 @@ namespace ThreatModelForge.Cli using System; using System.Collections.Generic; using System.Linq; - using System.Reflection; + using ThreatModelForge.Analysis; using ThreatModelForge.Editing; /// @@ -22,7 +22,7 @@ internal static class PropertiesCommand /// Zero on success; a non-zero value on error. public static int Run(string[] args) { - CliArgs parsed = CliArgs.Parse(args, new[] { "base" }, new[] { "explain" }); + CliArgs parsed = CliArgs.Parse(args, new[] { "base", "rules" }, new[] { "explain" }); if (parsed.Help) { PrintUsage(); @@ -56,7 +56,8 @@ public static int Run(string[] args) // The rules own the policy for each property (which rule reads it, at what severity, and // which values it flags). Join the pure schema with the loaded rule set at emit time so // severities and flagged values are never duplicated in the schema and cannot drift. - PropertyPolicyIndex policy = PropertyPolicyIndex.Build(new[] { Assembly.Load("ThreatModelForge.Analysis.Rules") }); + using RuleSet ruleSet = AnalysisRuleSources.Create(RuleSourceCli.FromPath(parsed.Get("rules"))); + PropertyPolicyIndex policy = PropertyPolicyIndex.Build(ruleSet); if (parsed.HasFlag("explain")) { @@ -191,7 +192,7 @@ private static void PrintUsage() { Console.Error.WriteLine("List the built-in typed property schema (custom properties the analyzer reads and Studio edits)."); Console.Error.WriteLine("Usage:"); - Console.Error.WriteLine(" tmforge properties [--base ] [--explain] [--json]"); + Console.Error.WriteLine(" tmforge properties [--base ] [--explain] [--json] [--rules ]"); Console.Error.WriteLine(); Console.Error.WriteLine("--explain maps each property VALUE to the rule ID and severity it triggers, so you can"); Console.Error.WriteLine("predict analyze behavior before running 'tmforge analyze'."); diff --git a/src/ThreatModelForge.Cli/PropertyPolicyIndex.cs b/src/ThreatModelForge.Cli/PropertyPolicyIndex.cs index 8c03412..d2bcbc7 100644 --- a/src/ThreatModelForge.Cli/PropertyPolicyIndex.cs +++ b/src/ThreatModelForge.Cli/PropertyPolicyIndex.cs @@ -2,7 +2,6 @@ namespace ThreatModelForge.Cli { using System; using System.Collections.Generic; - using System.Reflection; using ThreatModelForge.Analysis; /// @@ -21,13 +20,12 @@ private PropertyPolicyIndex(Dictionary byKey) this.byKey = byKey; } - /// Builds the index by reflecting each rule's property bindings. - /// The assemblies that contain the rules. + /// Builds the index from the rules in a loaded rule set (built-in plus any custom). + /// The rule set whose rules' property bindings are indexed. /// A populated index. - public static PropertyPolicyIndex Build(IEnumerable ruleAssemblies) + public static PropertyPolicyIndex Build(RuleSet ruleSet) { Dictionary map = new Dictionary(StringComparer.OrdinalIgnoreCase); - using RuleSet ruleSet = RuleSet.LoadDefault(ruleAssemblies); foreach (Rule rule in ruleSet.Rules) { string severity = rule.Severity.ToString(); diff --git a/src/ThreatModelForge.Cli/RuleSourceCli.cs b/src/ThreatModelForge.Cli/RuleSourceCli.cs new file mode 100644 index 0000000..3d8926c --- /dev/null +++ b/src/ThreatModelForge.Cli/RuleSourceCli.cs @@ -0,0 +1,30 @@ +namespace ThreatModelForge.Cli +{ + using System; + using ThreatModelForge.Analysis; + + /// + /// Builds from the CLI --rules option, wiring load and + /// validation warnings to standard error so they never corrupt machine-readable output on standard + /// out. Shared by the commands that load rules (analyze, threats, properties) so + /// custom rules behave identically across them. + /// + internal static class RuleSourceCli + { + /// The name of the CLI value option that points at declarative rule specs. + public const string OptionName = "rules"; + + /// + /// Builds options from a single --rules value (a spec file or a directory of specs). + /// + /// The value of --rules, or when omitted. + /// Options carrying the spec path (if any) and a standard-error diagnostics sink. + public static RuleSourceOptions FromPath(string? specPath) + { + Action diagnostics = message => Console.Error.WriteLine("warning: " + message); + return string.IsNullOrWhiteSpace(specPath) + ? new RuleSourceOptions(null, diagnostics) + : new RuleSourceOptions(new[] { specPath! }, diagnostics); + } + } +} diff --git a/src/ThreatModelForge.Cli/ThreatsCommand.cs b/src/ThreatModelForge.Cli/ThreatsCommand.cs index 4d4036e..f90e85f 100644 --- a/src/ThreatModelForge.Cli/ThreatsCommand.cs +++ b/src/ThreatModelForge.Cli/ThreatsCommand.cs @@ -30,7 +30,7 @@ public static int Run(string[] args) return 1; } - CliArgs parsed = CliArgs.Parse(args, Array.Empty(), new[] { "write" }); + CliArgs parsed = CliArgs.Parse(args, new[] { "rules" }, new[] { "write" }); if (parsed.Help) { PrintUsage(); @@ -58,7 +58,8 @@ public static int Run(string[] args) } (ThreatModel model, IThreatModelFormat? format) = CliModelLoader.Load(input!); - GenerationResult result = ThreatGenerator.Generate(model); + using RuleSet ruleSet = AnalysisRuleSources.Create(RuleSourceCli.FromPath(parsed.Get("rules"))); + GenerationResult result = ThreatGenerator.Generate(model, ruleSet); ApplyResult? written = null; if (parsed.HasFlag("write")) @@ -179,7 +180,7 @@ 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(" tmforge threats [--write] [--json] [--rules ] "); Console.Error.WriteLine(); Console.Error.WriteLine("--write persist the threats into the model's register (preserves prior triage)."); Console.Error.WriteLine(); diff --git a/src/ThreatModelForge.Engine/EngineService.cs b/src/ThreatModelForge.Engine/EngineService.cs index d4d48c9..cdf7f73 100644 --- a/src/ThreatModelForge.Engine/EngineService.cs +++ b/src/ThreatModelForge.Engine/EngineService.cs @@ -4,7 +4,6 @@ namespace ThreatModelForge.Engine using System.Collections.Generic; using System.IO; using System.Linq; - using System.Reflection; using System.Text; using System.Text.Json; using ThreatModelForge.Analysis; @@ -563,7 +562,7 @@ private static IReadOnlyList ResolveIds(string? entity, Dictionary + /// Unit tests for : the seam that composes the built-in rules with + /// any opt-in declarative rules. + /// + [TestClass] + public class AnalysisRuleSourcesTests + { + private const string ValidCustomSpec = + "{\"rules\":[{\"id\":\"ACME900\",\"severity\":\"error\",\"appliesTo\":\"datastore\"," + + "\"message\":\"x\",\"assert\":{\"property\":\"Encrypted\",\"present\":true}}]}"; + + /// + /// With no options, the seam loads the built-in first-party rules. + /// + [TestMethod] + public void CreateWithoutOptionsLoadsBuiltInRules() + { + using (RuleSet set = AnalysisRuleSources.Create()) + { + Assert.IsTrue(set.Rules.Count > 0); + Assert.IsTrue(set.Rules.Any(rule => string.Equals(rule.ID, "TM1016", StringComparison.Ordinal))); + } + } + + /// + /// A custom spec adds its rule alongside the built-in rules. + /// + [TestMethod] + public void CreateMergesCustomRules() + { + string path = WriteSpec(ValidCustomSpec); + try + { + int builtInCount; + using (RuleSet baseline = AnalysisRuleSources.Create()) + { + builtInCount = baseline.Rules.Count; + } + + using (RuleSet set = AnalysisRuleSources.Create(new RuleSourceOptions(new[] { path }))) + { + Assert.AreEqual(builtInCount + 1, set.Rules.Count); + Assert.IsTrue(set.Rules.Any(rule => string.Equals(rule.ID, "ACME900", StringComparison.Ordinal))); + } + } + finally + { + File.Delete(path); + } + } + + /// + /// A custom rule whose id collides with a built-in rule is dropped and reported, so the + /// built-in namespace always wins. + /// + [TestMethod] + public void CreateDropsCustomRuleWithDuplicateId() + { + string path = WriteSpec( + "{\"rules\":[{\"id\":\"TM1016\",\"severity\":\"error\",\"appliesTo\":\"datastore\"," + + "\"message\":\"x\",\"assert\":{\"property\":\"Encrypted\",\"present\":true}}]}"); + try + { + int builtInCount; + using (RuleSet baseline = AnalysisRuleSources.Create()) + { + builtInCount = baseline.Rules.Count; + } + + List diagnostics = new List(); + using (RuleSet set = AnalysisRuleSources.Create(new RuleSourceOptions(new[] { path }, diagnostics.Add))) + { + Assert.AreEqual(builtInCount, set.Rules.Count); + Assert.IsTrue(diagnostics.Any(message => message.Contains("TM1016"))); + } + } + finally + { + File.Delete(path); + } + } + + private static string WriteSpec(string json) + { + string path = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString("N") + ".tmrules.json"); + File.WriteAllText(path, json); + return path; + } + } +} diff --git a/test/ThreatModelForge.Analysis.Rules.Tests/DeclarativeRuleTests.cs b/test/ThreatModelForge.Analysis.Rules.Tests/DeclarativeRuleTests.cs new file mode 100644 index 0000000..102845a --- /dev/null +++ b/test/ThreatModelForge.Analysis.Rules.Tests/DeclarativeRuleTests.cs @@ -0,0 +1,219 @@ +namespace ThreatModelForge.Analysis.Rules.Tests +{ + using System; + using System.Collections.Generic; + using System.IO; + using Microsoft.VisualStudio.TestTools.UnitTesting; + using ThreatModelForge.KnowledgeBase; + using ThreatModelForge.Model; + using ThreatModelForge.Model.Abstracts; + + /// + /// Unit tests for declarative (data-defined) rules loaded through + /// and evaluated against a model. These verify that a declarative rule resolves properties, + /// trust-boundary crossings, and endpoint kinds identically to a compiled rule. + /// + [TestClass] + public class DeclarativeRuleTests + { + private const string StoreGenericTypeId = "GE.DS"; + + private const string ExternalGenericTypeId = "GE.EI"; + + private const string ProcessGenericTypeId = "GE.P"; + + /// + /// A datastore requirement flags a store that violates it and passes one that meets it. + /// + [TestMethod] + public void DataStoreAssertFlagsViolation() + { + string spec = "{\"rules\":[{\"id\":\"ACME001\",\"severity\":\"error\",\"appliesTo\":\"datastore\"," + + "\"message\":\"{name} is not encrypted\",\"assert\":{\"property\":\"Encrypted\",\"notAnyOf\":[\"No\"]}}]}"; + + StencilParallelLines unencrypted = CreateStore("DB", ("Encrypted", "No")); + MockMessageWriter flagged = Evaluate(spec, ModelWithComponents(unencrypted)); + Assert.AreEqual(1, flagged.Messages.Count); + Assert.AreSame(unencrypted, flagged.Messages[0].Target); + Assert.IsTrue(flagged.Messages[0].Text!.Contains("DB")); + + StencilParallelLines encrypted = CreateStore("DB", ("Encrypted", "At-rest")); + MockMessageWriter clean = Evaluate(spec, ModelWithComponents(encrypted)); + Assert.AreEqual(0, clean.Messages.Count); + } + + /// + /// A flow rule combining a property guard, a trust-boundary-crossing guard, and a protocol + /// requirement flags only a sensitive flow that crosses a boundary over a cleartext protocol. + /// + [TestMethod] + public void FlowCrossingBoundaryWithSensitiveDataRequiresEncryption() + { + string spec = "{\"rules\":[{\"id\":\"ACME002\",\"severity\":\"warning\",\"appliesTo\":\"flow\"," + + "\"message\":\"{name} sends sensitive data in the clear across a boundary\"," + + "\"when\":{\"property\":\"DataType\",\"anyOf\":[\"EUII\"],\"crossesTrustBoundary\":true}," + + "\"assert\":{\"property\":\"Protocol\",\"anyOf\":[\"HTTPS\",\"TLS\",\"mTLS\"]}}]}"; + + BorderBoundary border = CreateBoundary(); + + Connector cleartextCrossing = CreateFlow("Sync", crossing: true, ("DataType", "EUII"), ("Protocol", "HTTP")); + Assert.AreEqual(1, EvaluateFlow(spec, border, cleartextCrossing).Messages.Count); + + Connector encryptedCrossing = CreateFlow("Sync", crossing: true, ("DataType", "EUII"), ("Protocol", "HTTPS")); + Assert.AreEqual(0, EvaluateFlow(spec, border, encryptedCrossing).Messages.Count); + + Connector cleartextInside = CreateFlow("Sync", crossing: false, ("DataType", "EUII"), ("Protocol", "HTTP")); + Assert.AreEqual(0, EvaluateFlow(spec, border, cleartextInside).Messages.Count); + } + + /// + /// A flow rule guarded on the target endpoint's kind flags a flow to an external interactor and + /// ignores an otherwise-identical flow to an internal component. + /// + [TestMethod] + public void FlowToExternalTargetRequiresEncryption() + { + string spec = "{\"rules\":[{\"id\":\"ACME003\",\"severity\":\"warning\",\"appliesTo\":\"flow\"," + + "\"message\":\"{name} reaches an external party over an insecure channel\"," + + "\"when\":{\"target\":{\"kind\":\"external\"}}," + + "\"assert\":{\"property\":\"Protocol\",\"anyOf\":[\"HTTPS\"]}}]}"; + + StencilRectangle external = CreateExternal("Partner"); + Connector toExternal = FlowTo(external.Guid, "Push", ("Protocol", "HTTP")); + MockMessageWriter flagged = EvaluateEndpoint(spec, external, toExternal); + Assert.AreEqual(1, flagged.Messages.Count); + Assert.AreSame(toExternal, flagged.Messages[0].Target); + + StencilEllipse process = CreateProcess("Worker"); + Connector toProcess = FlowTo(process.Guid, "Push", ("Protocol", "HTTP")); + MockMessageWriter clean = EvaluateEndpoint(spec, process, toProcess); + Assert.AreEqual(0, clean.Messages.Count); + } + + /// + /// A rule with a guard and no requirement flags every element that matches the guard. + /// + [TestMethod] + public void GuardWithoutRequirementFlagsEveryMatch() + { + string spec = "{\"rules\":[{\"id\":\"ACME004\",\"severity\":\"info\",\"appliesTo\":\"flow\"," + + "\"message\":\"{name} crosses a trust boundary\"," + + "\"when\":{\"crossesTrustBoundary\":true}}]}"; + + BorderBoundary border = CreateBoundary(); + Assert.AreEqual(1, EvaluateFlow(spec, border, CreateFlow("Call", crossing: true)).Messages.Count); + Assert.AreEqual(0, EvaluateFlow(spec, border, CreateFlow("Call", crossing: false)).Messages.Count); + } + + private static MockMessageWriter Evaluate(string specJson, ThreatModel model) + { + string path = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString("N") + ".tmrules.json"); + File.WriteAllText(path, specJson); + try + { + IReadOnlyList rules = DeclarativeRuleProvider.Load(new[] { path }); + MockMessageWriter writer = new MockMessageWriter(); + RuleEvaluationContext context = new RuleEvaluationContext(model, writer); + foreach (Rule rule in rules) + { + rule.Evaluate(context); + } + + return writer; + } + finally + { + File.Delete(path); + } + } + + private static MockMessageWriter EvaluateFlow(string specJson, BorderBoundary border, Connector flow) + { + DrawingSurfaceModel diagram = new DrawingSurfaceModel { Header = "DFD-0" }; + diagram.Borders.Add(border.Guid, border); + diagram.Lines.Add(flow.Guid, flow); + return Evaluate(specJson, new ThreatModel { DrawingSurfaceList = { diagram } }); + } + + private static MockMessageWriter EvaluateEndpoint(string specJson, Entity endpoint, Connector flow) + { + DrawingSurfaceModel diagram = new DrawingSurfaceModel { Header = "DFD-0" }; + diagram.Borders.Add(endpoint.Guid, endpoint); + diagram.Lines.Add(flow.Guid, flow); + return Evaluate(specJson, new ThreatModel { DrawingSurfaceList = { diagram } }); + } + + private static ThreatModel ModelWithComponents(params Entity[] components) + { + DrawingSurfaceModel diagram = new DrawingSurfaceModel { Header = "DFD-0" }; + foreach (Entity component in components) + { + diagram.Borders.Add(component.Guid, component); + } + + return new ThreatModel { DrawingSurfaceList = { diagram } }; + } + + private static StencilParallelLines CreateStore(string name, params (string Name, string Value)[] properties) + { + StencilParallelLines store = new StencilParallelLines { Guid = Guid.NewGuid(), GenericTypeId = StoreGenericTypeId }; + store.Properties.Add(new StringDisplayAttribute { DisplayName = "Name", Value = name }); + AddProperties(store, properties); + return store; + } + + private static StencilRectangle CreateExternal(string name) + { + StencilRectangle external = new StencilRectangle { Guid = Guid.NewGuid(), GenericTypeId = ExternalGenericTypeId }; + external.Properties.Add(new StringDisplayAttribute { DisplayName = "Name", Value = name }); + return external; + } + + 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 BorderBoundary CreateBoundary() + { + return new BorderBoundary { Guid = Guid.NewGuid(), Left = 5, Top = 10, Height = 15, Width = 20 }; + } + + private static Connector CreateFlow(string name, bool crossing, params (string Name, string Value)[] properties) + { + // The target sits inside the boundary (x in [5,25), y in [10,25)); the source sits inside + // when the flow stays local and outside when it crosses, so Crosses() is source XOR target. + Connector flow = new Connector + { + Guid = Guid.NewGuid(), + SourceGuid = Guid.NewGuid(), + TargetGuid = Guid.NewGuid(), + SourceX = crossing ? 0 : 10, + SourceY = crossing ? 0 : 11, + TargetX = 12, + TargetY = 15, + }; + flow.Properties.Add(new StringDisplayAttribute { DisplayName = "Name", Value = name }); + AddProperties(flow, properties); + return flow; + } + + private static Connector FlowTo(Guid target, string name, params (string Name, string Value)[] properties) + { + Connector flow = new Connector { Guid = Guid.NewGuid(), SourceGuid = Guid.NewGuid(), TargetGuid = target }; + flow.Properties.Add(new StringDisplayAttribute { DisplayName = "Name", Value = name }); + AddProperties(flow, properties); + return flow; + } + + private static void AddProperties(Entity entity, (string Name, string Value)[] properties) + { + foreach ((string propertyName, string propertyValue) in properties) + { + entity.Properties.Add(new CustomStringDisplayAttribute { Value = $"{propertyName}:{propertyValue}" }); + } + } + } +} diff --git a/test/ThreatModelForge.Analysis.Tests/AbstractRuleFixture.cs b/test/ThreatModelForge.Analysis.Tests/AbstractRuleFixture.cs new file mode 100644 index 0000000..5779d63 --- /dev/null +++ b/test/ThreatModelForge.Analysis.Tests/AbstractRuleFixture.cs @@ -0,0 +1,17 @@ +namespace ThreatModelForge.Analysis.Tests +{ + /// + /// A test fixture: an abstract subclass. It cannot be instantiated, so rule + /// discovery must skip it rather than throw when it appears in a scanned assembly. + /// + public abstract class AbstractRuleFixture : Rule + { + /// + /// Initializes a new instance of the class. + /// + protected AbstractRuleFixture() + : base(9001, MessageSeverity.Warning, "Test") + { + } + } +} diff --git a/test/ThreatModelForge.Analysis.Tests/DeclarativeRuleProviderTests.cs b/test/ThreatModelForge.Analysis.Tests/DeclarativeRuleProviderTests.cs new file mode 100644 index 0000000..7b3e869 --- /dev/null +++ b/test/ThreatModelForge.Analysis.Tests/DeclarativeRuleProviderTests.cs @@ -0,0 +1,155 @@ +namespace ThreatModelForge.Analysis.Tests +{ + using System; + using System.Collections.Generic; + using System.IO; + using System.Linq; + using Microsoft.VisualStudio.TestTools.UnitTesting; + + /// + /// Unit tests for the class. + /// + [TestClass] + public class DeclarativeRuleProviderTests + { + /// + /// Gets or sets the working directory created for each test. + /// + private string WorkingDirectory { get; set; } = string.Empty; + + /// + /// Creates an isolated working directory for the test. + /// + [TestInitialize] + public void Initialize() + { + this.WorkingDirectory = Path.Combine(Path.GetTempPath(), "tmforge-rules-" + 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); + } + } + + /// + /// A valid spec loads a single rule whose id is surfaced verbatim (the string-id constructor) + /// and whose severity, pack, STRIDE category, and external references are parsed. + /// + [TestMethod] + public void LoadsValidRuleWithMetadata() + { + string spec = + "{\"rules\":[{" + + "\"id\":\"ACME001\",\"pack\":\"acme\",\"severity\":\"error\"," + + "\"appliesTo\":\"datastore\",\"message\":\"{name} is not encrypted\"," + + "\"stride\":\"InformationDisclosure\",\"threatReferences\":[\"CWE:311\"]," + + "\"assert\":{\"property\":\"Encrypted\",\"notAnyOf\":[\"No\"]}}]}"; + string path = this.WriteSpec(spec); + + IReadOnlyList rules = DeclarativeRuleProvider.Load(new[] { path }); + + Assert.AreEqual(1, rules.Count); + Rule rule = rules[0]; + Assert.AreEqual("ACME001", rule.ID); + Assert.AreEqual("acme", rule.Pack); + Assert.AreEqual(MessageSeverity.Error, rule.Severity); + Assert.AreEqual(StrideCategory.InformationDisclosure, rule.Stride); + Assert.AreEqual(1, rule.ThreatReferences.Count); + Assert.AreEqual("CWE-311", rule.ThreatReferences[0].Id); + } + + /// + /// A rule without an id is skipped and reported through diagnostics. + /// + [TestMethod] + public void SkipsRuleMissingId() + { + string spec = "{\"rules\":[{\"appliesTo\":\"datastore\",\"message\":\"x\",\"assert\":{\"property\":\"Encrypted\",\"present\":true}}]}"; + string path = this.WriteSpec(spec); + List diagnostics = new List(); + + IReadOnlyList rules = DeclarativeRuleProvider.Load(new[] { path }, diagnostics.Add); + + Assert.AreEqual(0, rules.Count); + Assert.IsTrue(diagnostics.Any(message => message.Contains("id"))); + } + + /// + /// A rule with an unrecognized appliesTo is skipped and reported. + /// + [TestMethod] + public void SkipsUnknownAppliesTo() + { + string spec = "{\"rules\":[{\"id\":\"X1\",\"appliesTo\":\"widget\",\"message\":\"x\",\"assert\":{\"property\":\"P\",\"present\":true}}]}"; + string path = this.WriteSpec(spec); + List diagnostics = new List(); + + IReadOnlyList rules = DeclarativeRuleProvider.Load(new[] { path }, diagnostics.Add); + + Assert.AreEqual(0, rules.Count); + Assert.IsTrue(diagnostics.Any(message => message.Contains("appliesTo"))); + } + + /// + /// Relational facets are only valid on flow rules; using one on a non-flow rule is rejected. + /// + [TestMethod] + public void SkipsRelationalConditionOnNonFlow() + { + string spec = "{\"rules\":[{\"id\":\"X1\",\"appliesTo\":\"datastore\",\"message\":\"x\",\"when\":{\"crossesTrustBoundary\":true}}]}"; + string path = this.WriteSpec(spec); + List diagnostics = new List(); + + IReadOnlyList rules = DeclarativeRuleProvider.Load(new[] { path }, diagnostics.Add); + + Assert.AreEqual(0, rules.Count); + Assert.IsTrue(diagnostics.Any(message => message.Contains("flow"))); + } + + /// + /// A rule with neither a guard nor a requirement is rejected. + /// + [TestMethod] + public void SkipsRuleWithoutWhenOrAssert() + { + string spec = "{\"rules\":[{\"id\":\"X1\",\"appliesTo\":\"process\",\"message\":\"x\"}]}"; + string path = this.WriteSpec(spec); + List diagnostics = new List(); + + IReadOnlyList rules = DeclarativeRuleProvider.Load(new[] { path }, diagnostics.Add); + + Assert.AreEqual(0, rules.Count); + Assert.IsTrue(diagnostics.Any(message => message.Contains("when") || message.Contains("assert"))); + } + + /// + /// A malformed spec file is skipped with a diagnostic rather than throwing. + /// + [TestMethod] + public void SkipsMalformedJson() + { + string path = this.WriteSpec("{ this is not valid json"); + List diagnostics = new List(); + + IReadOnlyList rules = DeclarativeRuleProvider.Load(new[] { path }, diagnostics.Add); + + Assert.AreEqual(0, rules.Count); + Assert.AreEqual(1, diagnostics.Count); + } + + private string WriteSpec(string json) + { + string path = Path.Combine(this.WorkingDirectory, Guid.NewGuid().ToString("N") + ".tmrules.json"); + File.WriteAllText(path, json); + return path; + } + } +} diff --git a/test/ThreatModelForge.Analysis.Tests/NoDefaultCtorRuleFixture.cs b/test/ThreatModelForge.Analysis.Tests/NoDefaultCtorRuleFixture.cs new file mode 100644 index 0000000..64924af --- /dev/null +++ b/test/ThreatModelForge.Analysis.Tests/NoDefaultCtorRuleFixture.cs @@ -0,0 +1,25 @@ +namespace ThreatModelForge.Analysis.Tests +{ + /// + /// A test fixture: a concrete with no public parameterless constructor. Rule + /// discovery must skip it (reporting a diagnostic) rather than throw when it appears in a scanned + /// assembly. + /// + public sealed class NoDefaultCtorRuleFixture : Rule + { + /// + /// Initializes a new instance of the class. + /// + /// A required argument, so the type has no public parameterless constructor. + public NoDefaultCtorRuleFixture(string tag) + : base("NOCTOR-1", MessageSeverity.Info, "Test") + { + _ = tag; + } + + /// + public override void Evaluate(RuleEvaluationContext context) + { + } + } +} diff --git a/test/ThreatModelForge.Analysis.Tests/RuleSetTests.cs b/test/ThreatModelForge.Analysis.Tests/RuleSetTests.cs index b903f6b..6b9b572 100644 --- a/test/ThreatModelForge.Analysis.Tests/RuleSetTests.cs +++ b/test/ThreatModelForge.Analysis.Tests/RuleSetTests.cs @@ -65,6 +65,25 @@ public void LoadTest() Assert.AreEqual(true, actual2!.Disabled); } + /// + /// Discovery is resilient to rule types that cannot be instantiated: an abstract rule subclass + /// () and a rule without a public parameterless constructor + /// () are skipped and reported through diagnostics rather + /// than aborting the load, so the valid rules in the same assembly still load. + /// + [TestMethod] + public void LoadDefaultSkipsUninstantiableRules() + { + List diagnostics = new List(); + using (RuleSet target = RuleSet.LoadDefault(new[] { this.GetType().Assembly }, diagnostics.Add)) + { + Assert.IsTrue(target.Rules.Any(rule => string.Equals(rule.ID, "TM1234", StringComparison.Ordinal))); + Assert.IsTrue(target.Rules.Any(rule => string.Equals(rule.ID, "TM1235", StringComparison.Ordinal))); + Assert.IsFalse(target.Rules.Any(rule => rule is NoDefaultCtorRuleFixture)); + Assert.IsTrue(diagnostics.Any(message => message.Contains("NoDefaultCtorRuleFixture"))); + } + } + /// /// Unit test for the method. /// diff --git a/test/ThreatModelForge.Cli.Tests/CustomRulesCliTest.cs b/test/ThreatModelForge.Cli.Tests/CustomRulesCliTest.cs new file mode 100644 index 0000000..653dc7d --- /dev/null +++ b/test/ThreatModelForge.Cli.Tests/CustomRulesCliTest.cs @@ -0,0 +1,128 @@ +namespace ThreatModelForge.Cli.Tests +{ + using System; + using System.IO; + using Microsoft.VisualStudio.TestTools.UnitTesting; + + /// + /// End-to-end tests for the --rules option on analyze: a declarative custom rule is + /// loaded, evaluated, and gates the exit code alongside the built-in rules. + /// + [TestClass] + public class CustomRulesCliTest + { + private const string ModelAllPacksDisabled = + "{\"schema\":\"tmforge-json\",\"version\":\"0.1\"," + + "\"elements\":[" + + "{\"id\":\"p1\",\"kind\":\"process\",\"name\":\"API\",\"x\":100,\"y\":100}," + + "{\"id\":\"ds1\",\"kind\":\"datastore\",\"name\":\"DB\",\"x\":400,\"y\":100}]," + + "\"flows\":[{\"id\":\"f1\",\"source\":\"p1\",\"target\":\"ds1\",\"name\":\"query\"}]," + + "\"analysis\":{\"disabledPacks\":[\"core-hygiene\",\"stride-completeness\",\"input-validation\",\"data-protection\",\"transport-security\",\"identity-access\"]}}"; + + private const string DataStoreEncryptionSpec = + "{\"rules\":[{\"id\":\"ACME900\",\"severity\":\"error\",\"appliesTo\":\"datastore\"," + + "\"message\":\"{name} must declare encryption\",\"assert\":{\"property\":\"Encrypted\",\"present\":true}}]}"; + + private const string ExternalOnlySpec = + "{\"rules\":[{\"id\":\"ACME901\",\"severity\":\"error\",\"appliesTo\":\"external\"," + + "\"message\":\"{name} must authenticate\",\"assert\":{\"property\":\"AuthenticatesItself\",\"equals\":\"Yes\"}}]}"; + + /// + /// Gets or sets the working directory created for each test. + /// + private string WorkingDirectory { get; set; } = string.Empty; + + /// + /// Creates an isolated working directory for the test. + /// + [TestInitialize] + public void Initialize() + { + this.WorkingDirectory = Path.Combine(Path.GetTempPath(), "tmforge-customrules-" + 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); + } + } + + /// + /// A custom error-severity rule that fires on the model gates the exit code (2), even though + /// every built-in rule pack is disabled by the model. + /// + [TestMethod] + public void CustomRuleFiresViaRulesOption() + { + string model = this.Write("model.tmforge.json", ModelAllPacksDisabled); + string spec = this.Write("rules.tmrules.json", DataStoreEncryptionSpec); + + (int exit, _) = Run(new[] { "--rules", spec, model }); + + Assert.AreEqual(2, exit); + } + + /// + /// Without --rules, the same model analyzes clean (exit 0) because its built-in packs + /// are disabled and no custom rule is loaded. + /// + [TestMethod] + public void WithoutRulesOptionModelIsClean() + { + string model = this.Write("model.tmforge.json", ModelAllPacksDisabled); + + (int exit, _) = Run(new[] { model }); + + Assert.AreEqual(0, exit); + } + + /// + /// A custom rule that matches no element in the model does not gate the exit code, proving the + /// rule is actually evaluated rather than firing unconditionally. + /// + [TestMethod] + public void CustomRuleThatMatchesNothingKeepsExitZero() + { + string model = this.Write("model.tmforge.json", ModelAllPacksDisabled); + string spec = this.Write("rules.tmrules.json", ExternalOnlySpec); + + (int exit, _) = Run(new[] { "--rules", spec, model }); + + Assert.AreEqual(0, exit); + } + + private static (int Exit, string Stdout) Run(string[] args) + { + 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 = AnalyzeCommand.Run(args); + return (exit, outWriter.ToString()); + } + finally + { + Console.SetOut(originalOut); + Console.SetError(originalError); + } + } + + private string Write(string fileName, string content) + { + string path = Path.Combine(this.WorkingDirectory, fileName); + File.WriteAllText(path, content); + return path; + } + } +}