Boolean expression support in the rules engine

[Giulia Stocco (gfs)]

Adds boolean expression support to the rules engine, so a rule can be described as an explicit expression over named patterns and conditions instead of the single hardcoded (P0 OR P1 ...) AND C0 AND C1 ... shape. Along the way it fixes several bugs in the surrounding code.

Closes #524.
Supersedes #650 — that branch is now the base of this one, so #650 can be closed unmerged. #652 was merged into #650 and comes along with it.

Relationship to #650

#650 fixed WithinOperation returning after its first capture, and added pattern level conditions. This branch needed the same code, and the two fixes overlapped enough that landing them separately would have meant resolving the same conflicts twice. So #650's two commits are replayed onto main first and this work is rebased on top, giving one reviewable history.

Where the two designs collided, the rule was: #650 wins on condition mechanics, this PR wins on expressions.

• Fix conditions dropping all but the first pattern's findings #650's GovernedMatches fix is kept over the deduplication approach originally used here; that commit is now tests only.
• Fix conditions dropping all but the first pattern's findings #650's pattern.conditions is kept for scoping a condition to one pattern. The applies_to_patterns field originally proposed here is dropped entirely — the two solved the same problem and pattern.conditions needs no new label resolution. Its tests were rewritten against nested conditions, so the capability stays covered.
• FilterCaptures combines both: Fix conditions dropping all but the first pattern's findings #650's gate semantics inside the sync/async unification from this branch.
• The schema's search_in combines both: Fix conditions dropping all but the first pattern's findings #650 correctly removed file (never handled by GenerateCondition), and the finding-region constraints from this branch sit on top.

Reviewing this

It is a big diff, so the commits are ordered to be read in sequence and each one builds and passes on its own.

Commits 1, 2, 4, 7 and 12 are standalone bug fixes with no dependency on the feature.

#	Commit	Kind
1	Fix conditions dropping all but the first pattern's findings	bug fix (#650)
2	Make pattern-level conditions actually gate their pattern (#652)	bug fix (#650)
3	Add expression and label to the rule model	model + schema
4	Report the pattern that actually matched for string patterns	bug fix
5	Pin the expression semantics the rules engine depends on	tests only
6	Add regression tests for conditions across multiple pattern captures	tests only
7	Unify the synchronous and asynchronous analysis paths	bug fix
8	Translate author supplied labels and expressions	feature
9	Validate rule expressions and labels	feature
10	Report the findings that satisfy the rule expression	feature
11	Document rule expressions	docs
12	Apply rule overrides consistently in both analysis paths	bug fix
13	Share one implementation between the sync and async analysis paths	refactor
14	Take OAT 1.2.95 for the parenthesis balance fix	dependency
15	Bump minor version for the rules engine work	release
16–26	Review response: schema alignment, nesting bounds, defect fixes, hardening, docs	review

The highest risk commit is 10, because it changes which findings get reported. The legacy path is untouched when a rule has no expression.

What rule authors get

Two independent additions, either usable alone.

conditions on a pattern (from #650) scopes a condition to the pattern it guards, so guarding one pattern no longer requires splitting the rule in two:

"patterns": [
  {
    "pattern": "curl", "type": "substring", "label": "curl",
    "conditions": [
      { "pattern": { "pattern": "--tlsv1.3", "type": "substring" },
        "search_in": "same-line", "negate_finding": true }
    ]
  },
  { "pattern": "wget", "type": "substring", "label": "wget" }
]

expression combines labels with AND, OR, XOR, NAND, NOR, NOT:

"expression": "cookie AND NOT (secure AND httponly)"

That one fires when either required flag is missing. Expressed as two negated conditions it would instead mean "neither flag is present", which stays silent on partially hardened code, the common real world defect.

Bugs fixed

From #650:

1. Multi pattern rules with a condition reported only the first pattern's findings. WithinOperation returned from inside its loop over captures. Seven shipped rules are under reporting today: AI016300, AI036000, AI036622, AI038210, AI038500, AI080001, AI084000.
2. Pattern level conditions did not gate their pattern (Make pattern-level conditions actually gate their pattern #652).

From this branch:

3. String and substring patterns always reported pattern index 0. Each becomes its own clause holding a single element of Data, so deriving the index from the position in that list always gave 0. MatchingPattern and confidence filtering resolved against the wrong pattern.
4. The async path over reported. AnalyzeFileAsync took the union of within clause captures instead of intersecting them, so it emitted duplicates and findings the conditions should have excluded. It also never populated StartLocationColumn/EndLocationColumn and had an off by one bounds check.
5. Overrides used different overlap rules per path. Sync required only that the overridden finding start inside the overriding one; async required containment. Settled on containment, which is what the sync code's own comment described.
6. One unreadable file abandoned the entire scan. Two per file paths had no exception handling, so a single failure propagated out of the scan loop. Each file now fails to its own ScanState.Error record and the scan continues.

Commit 13 then removes the duplicated scan loop that allowed 4 and 5 to happen, so both entry points share one implementation.

Behaviour changes to be aware of

Existing rule JSON is unchanged and continues to validate, but results are not bit identical, because the bug fixes correct which findings are reported:

• multi pattern rules with conditions no longer drop findings, affecting the seven default rules listed above (more results)
• string pattern matches now resolve to the pattern that actually matched
• the async path no longer over reports (fewer results)
• overrides now require containment (previously suppressed findings may return)

No rule shipped with Application Inspector uses overrides, so default scan output is unaffected by that last one. Anyone baselining scan output should expect a diff. This is why the version goes to 1.10 rather than a patch; the new public API on the rules engine is additive and independently forces a minor.

Schema stays rule-schema-v1.json since every addition is backward compatible; it carries a "version": "1.1.0" annotation.

Verification

• 491 tests pass on net9.0 and net10.0; full solution builds clean including netstandard2.1
• verifyrules reports zero failures across the shipped rule set
• every one of the 26 commits was independently built and tested in a clean worktree, growing 354 to 491 tests
• each bug fix was confirmed by reverting the fix and watching the new test fail
• a golden test asserts the generated expression string is byte for byte unchanged for every shipped rule
• OatExpressionSemanticsTests pins the engine behaviour the translation relies on: no operator precedence, parentheses group and inherit accumulated captures, duplicate labels evaluate false

Notes for the reviewer

• Expressions have no operator precedence and fold strictly left to right, so a OR b AND c means (a OR b) AND c. Verification rejects an expression that mixes operators at one level without parentheses, which should stop that being a footgun in practice.
• Evaluating an unbalanced expression still throws even on OAT 1.2.95, which only fixed the validation gap (Add validation for unbalanced parentheses in rules OAT#393). Verification is therefore the gate that keeps a malformed rule away from the analyzer, and RulesVerifier keeps its own balance check.
• Per-finding expression evaluation earns its place on one case: sibling conditions each satisfied by a different finding, e.g. p AND (g1 OR g2) where g1 and g2 match on separate lines. OAT's accumulated captures report nothing there. ConditionsSatisfiedByDifferentFindings_ReportBoth is the single test that fails if that filtering is removed.
• A rule carries two expressions. The engine gets a plain disjunction of every clause, which only decides whether a file is worth examining; the authored expression stays on the rule and decides, per finding, what is reported. Handing the authored expression to the engine instead evaluates NOT across the whole file, so one compliant finding suppresses vulnerable siblings — NegationIsJudgedPerFinding_NotAcrossTheFile covers this. A consequence is that the engine never sees the authored operands, so verification checks them against the rule's own labels rather than relying on OAT.
• Still outstanding: DevSkim has not been built against this. Worth checking its rule set verifies unchanged, and whether any of its rules use overrides.

[Copilot]

Pull request overview

This pull request extends Application Inspector’s rules engine to support author-defined boolean expressions over named pattern/condition labels, and adds applies_to_patterns to scope conditions to subsets of patterns. It also unifies sync/async analysis paths and includes multiple correctness fixes plus new tests that pin the intended semantics.

Changes:

• Add expression, label, and applies_to_patterns support end-to-end (schema, model, translation, verification, evaluation).
• Fix several matching/reporting issues (pattern index reporting, condition evaluation across all captures, sync/async parity, override containment).
• Add comprehensive xUnit coverage for expression parsing/behavior/validation and regression scenarios; bump OAT and minor version.

Show a summary per file

File	Description
version.json	Bumps minor version to reflect behavior/output changes and new additive API surface.
rule-schema-v1.json	Updates rule schema for expression, label, applies_to_patterns, and new search_in options.
AppInspector/rules/samples/README.md	Documents expressions, labels, scoping, and authoring rules.
AppInspector/AppInspector.Commands.csproj	Bumps Microsoft.CST.OAT to 1.2.95.
AppInspector.RulesEngine/AppInspector.RulesEngine.csproj	Bumps Microsoft.CST.OAT to 1.2.95 in the engine project.
AppInspector.RulesEngine/SearchPattern.cs	Adds label to patterns for expression authoring.
AppInspector.RulesEngine/SearchCondition.cs	Adds label and applies_to_patterns to conditions.
AppInspector.RulesEngine/Rule.cs	Adds expression field to the rule model.
AppInspector.RulesEngine/RulesVerifier.cs	Adds verification for label validity, duplicates, applies_to scoping, and expression safety constraints.
AppInspector.RulesEngine/RuleProcessor.cs	Unifies sync/async analysis into one implementation; adds per-finding expression filtering; standardizes override removal.
AppInspector.RulesEngine/RuleExpression.cs	New internal parser/evaluator mirroring the engine’s left-to-right folding semantics.
AppInspector.RulesEngine/AbstractRuleSet.cs	Translates labels/expressions into OAT clauses and expressions; wires condition scoping.
AppInspector.RulesEngine/OatExtensions/WithinOperation.cs	Fixes within evaluation to consider all captures and supports pattern-scoped conditions.
AppInspector.RulesEngine/OatExtensions/WithinClause.cs	Adds AppliesToPatternIndices and guarding logic.
AppInspector.RulesEngine/OatExtensions/OatSubstringIndexOperation.cs	Fixes string/substring pattern index reporting.
AppInspector.RulesEngine/OatExtensions/OatSubstringIndexClause.cs	Adds PatternIndex to identify originating pattern.
AppInspector.RulesEngine/OatExtensions/OatRegexWithIndexOperation.cs	Switches regex index reporting to PatternIndex instead of clause.Label.
AppInspector.RulesEngine/OatExtensions/OatRegexWithIndexClause.cs	Adds PatternIndex to identify originating pattern.
AppInspector.Tests/RuleProcessor/WithinClauseMultiPatternTests.cs	New regression tests for condition evaluation across multiple pattern captures.
AppInspector.Tests/RuleProcessor/SyncAsyncParityTests.cs	New parity tests ensuring sync/async results match, including cancellation behavior.
AppInspector.Tests/RuleProcessor/RuleExpressionParserTests.cs	New tests for parsing and evaluation semantics (L2R folding, parentheses, malformed expressions).
AppInspector.Tests/RuleProcessor/RuleExpressionBehaviourTests.cs	New functional tests for expression-only and applies_to_patterns-only scenarios.
AppInspector.Tests/RuleProcessor/PatternIndexReportingTests.cs	New regression tests for correct pattern index reporting + confidence filtering.
AppInspector.Tests/RuleProcessor/OverrideTests.cs	New tests pinning override containment semantics and sync/async agreement.
AppInspector.Tests/RuleProcessor/OatExpressionSemanticsTests.cs	Characterization tests pinning OAT behaviors the translation depends on.
AppInspector.Tests/RuleProcessor/ExpressionValidationTests.cs	New tests asserting verification rejects malformed/unsafe authoring patterns.
AppInspector.Tests/RuleProcessor/ExpressionGenerationTests.cs	Golden tests ensuring legacy-generated expressions remain byte-for-byte stable for shipped rules.

Review details

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

• Files reviewed: 27/27 changed files
• Comments generated: 3
• Review effort level: Lite

[Giulia Stocco (gfs)]

Copilot Generated COMPASS SDL Security Review:

SDL Security Review

Scope: PR #654 feature/rule-boolean-expressions → main ("Boolean expression support in the rules engine"). Application source only; tests, README, version.json reviewed and clean. Deps are exact-pinned (Microsoft.CST.OAT 1.2.87 → 1.2.95) — no issue.

Note: git/gh-via-git are blocked by the active sandbox policy (/Users/giulia/Library/Caches/github-copilot-git-2.53.0-4/etc/gitconfig). I used the GitHub REST API for the diff. Consider allowing that path via /sandbox.

---

🔴 Blocking

AppInspector.RulesEngine/RuleExpression.cs — unbounded parser recursion (uncatchable stack exhaustion)

ParseSequence and ParseTerm are mutually recursive with no depth limit; recursion depth equals parenthesis nesting depth:

if (tokens[index].Kind == TokenKind.OpenParen)
{
    index++;
    inner = ParseSequence(tokens, ref index);   // line 140 -> ParseTerm -> ParseSequence ...

Tokenize (lines 61–72) happily emits one OpenParen token per ( character within a single whitespace-delimited token, so a rule containing "expression": "((((…(0)…))))" yields tens of thousands of nested frames.

Nothing in the chain bounds this:

• rule-schema-v1.json declares expression as a bare {"type": "string"} — no maxLength, no pattern.
• RulesVerifier.ValidateExpression (added in this PR) checks only balance (depth != 0) and mixed operators. Deeply nested but balanced input with a single label and no operators passes cleanly.
• AbstractRuleSet.cs:159 forwards the raw string verbatim:

  Expression = string.IsNullOrWhiteSpace(rule.Expression) ? expression.ToString() : rule.Expression

Attack vector / impact: an attacker who controls a rule file — a third-party rule pack (the repo ships packrules for exactly this distribution model), a -r/--custom-rules-path input, or rules supplied to the AppInspector.RulesEngine NuGet library by a hosting service — triggers StackOverflowException. In .NET Core this exception cannot be caught; it terminates the process immediately, bypassing every try/catch in the analyze pipeline. For an embedding scan service this is an unauthenticated, one-shot, full-process DoS costing ~200 KB of input. --disable-custom-rule-verification removes even the partial verifier gate.

Fix: thread a depth counter through ParseSequence/ParseTerm and return null past a cap (e.g. 64); add a matching nesting/length bound in ValidateExpression and a maxLength to the schema so this is rejected at load time rather than at evaluation time.

Assumption stated explicitly: this is 🔴 on the basis that rule content can originate outside the operator's trust boundary. If rule files are strictly trusted-operator-only in your threat model, downgrade to 🟡 — the fix is cheap either way.

---

🟡 Warning

AppInspector.RulesEngine/RuleExpression.cs:20,34 — unbounded process-wide cache

private static readonly ConcurrentDictionary<string, RuleExpression?> Cache = new();
...
return Cache.GetOrAdd(expression, static toParse => { ... });

Static, never evicted, keyed on the raw expression string, and it also stores null for every malformed expression. It outlives any RuleSet instance, so a long-running host that loads many or repeated rulesets grows memory monotonically, with the key material controlled by rule content. Bound the cache (size-capped/LRU) or scope it to the rule set.

AppInspector.RulesEngine/RuleProcessor.cs:264 + RulesVerifier.ValidateExpression — silent fail-open on bad expressions

if (oatRule.Expression is null || RuleExpression.TryParse(oatRule.Expression) is not { } expression)
{
    return new List<(int, Boundary)>();
}

A malformed expression discards every finding for that rule and logs nothing — the rule silently reports zero results even though the engine considered it matched. Relatedly, ValidateExpression verifies clause labels, duplicates, and applies_to_patterns targets, but never checks that labels referenced by the expression resolve to a real clause. An unresolved label evaluates to false via reportedByLabel.TryGetValue(...), so a single typo silently disables a detection rule (or, under NOT, silently makes it always-true).

In a security-analysis tool a silently disabled rule is a fail-open detection gap. Log an error in the TryParse-failed branch, and add unresolved-label detection to ValidateExpression for parity with the existing applies_to_patterns check.

---

Reviewed and clear

• OatSubstringIndexOperation / OatRegexWithIndexOperation / OatRegexWithIndexClause / OatSubstringIndexClause: replacing Convert.ToInt32(clause.Label) with an explicit PatternIndex removes a parse-failure path; BuildMatchRecord still range-checks patternIndex before indexing Patterns[].
• WithinOperation / WithinClause: Guards() defaults to null (guards everything), so existing rules keep prior semantics.
• RemoveOverriddenMatches: containment logic is byte-identical to main; the PR only de-duplicates the sync/async copies. Its O(n²) behavior is pre-existing, not a changed-line finding.
• Schema: pattern/condition label correctly constrained to ^[^\s()]+$.
• No secrets, crypto, injection, path-traversal, SSRF, auth, or cookie changes in this diff.

---

REQUEST CHANGES

[Copilot]

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

AppInspector.RulesEngine/RulesVerifier.cs:495

• This validation does not enforce the documented requirement that a condition be evaluated only after a pattern has supplied a finding. An expression such as c AND p can verify successfully, but WithinOperation receives no prior captures for c, so the rule silently never matches. Validate expression order/control flow so every reachable condition term has a preceding pattern source.

        var expression = rule.Expression!;

• Files reviewed: 30/30 changed files
• Comments generated: 4
• Review effort level: Balanced

[Copilot]

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

AppInspector.RulesEngine/AbstractRuleSet.cs:188

• An explicitly empty applies_to_patterns list is currently treated like omission because this guard requires Count > 0. The schema says omission applies the condition to every pattern, whereas [] names no guarded patterns; with the current code, adding an empty list unexpectedly makes the condition guard all patterns. Initialize an empty set whenever the property is present so omission and an explicit empty scope retain distinct meanings.

                if (condition.AppliesToPatterns is { Count: > 0 })

rule-schema-v1.json:277

• Adding only-before to the schema exposes an existing off-by-one in WithinOperation: its boundary ends at finding.Index and then adds one to the length, so the first character of the finding is included in the supposedly before-only search. A condition matching that character can therefore pass even when it occurs only inside the finding. Fix the boundary to exclude the finding before accepting this value in the schema.

          "pattern": "^(file|finding-region\\((0,\\s*[1-9]\\d*|-\\d+,\\s*\\d+)\\)|finding-only|same-line|same-file|only-before|only-after)$",

• Files reviewed: 30/30 changed files
• Comments generated: 2
• Review effort level: Balanced

[Copilot]

Review details

Suppressed comments (4)

Previously missed (1) — in code that hasn't changed since the last review.

AppInspector.RulesEngine/AbstractRuleSet.cs:188

• An explicitly empty applies_to_patterns array takes this false branch, leaving appliesTo as null; WithinClause.Guards defines null as guarding every pattern. That makes [] behave like omission even though the API and schema describe only omission as the all-pattern default. Preserve a non-null empty set so an empty list guards no patterns.

                if (condition.AppliesToPatterns is { Count: > 0 })

AppInspector.RulesEngine/RuleProcessor.cs:162

• The authored expression is still evaluated by OAT before this loop, at rule/file scope. For p AND NOT (a AND b), if one p finding has a and another has b, OAT sees both conditions as true and suppresses the entire RuleCapture; FilterCapturesByExpression never receives either candidate even though each individually satisfies the expression. Candidate capture collection must not be gated by the aggregate authored expression—collect the relevant clause captures independently, then apply RuleExpression per finding.

        foreach (var ruleCapture in _analyzer.GetCaptures(rules, textContainer))

AppInspector.RulesEngine/RulesVerifier.cs:649

• This silently disables the “can report a finding” validation for any rule declaring more than 16 conditions, even when those conditions are unused. For example, such a rule with a AND b then verifies successfully, but the per-finding evaluator can never make both originating pattern labels true and reports nothing. Either reject expressions whose satisfiability cannot be checked within this bound or use a bounded symbolic approach; do not mark them verified without running the check.

        if (conditionLabels.Count > maxConditionsToEnumerate)
        {
            yield break;
        }

AppInspector.RulesEngine/RulesVerifier.cs:544

• Expression operands are never checked strictly against the declared labels. OAT treats an unmatched numeric operand as a clause-list index, while the per-finding evaluator only uses actual labels. For patterns labeled foo and 1, the expression 0 OR 1 therefore verifies and OAT captures both patterns, but FilterCapturesByExpression treats 0 as false and silently drops every foo finding. Validate every operand against patternLabels/conditionLabels directly instead of accepting OAT's numeric fallback.

        var tokens = expression.Split(new[] { ' ' }, StringSplitOptions.RemoveEmptyEntries);

• Files reviewed: 32/32 changed files
• Comments generated: 1
• Review effort level: Balanced