Expression evaluation security review with non-subtractive guardrails (#500)

[JayVDZ]

Summary

Closes the "DynamicExpresso input path review" gap from the OWASP Top 10:2025 assessment (A03/A05, gap 5). Part of #500.

The deliverable is a documented security review, engineering/EXPRESSION_SECURITY.md, plus two strictly non-subtractive guardrails. Per the product owner's constraint, nothing in this PR changes what any legitimate expression can do; a 46-test functionality regression suite (expressions taken verbatim from the customer docs, covering every built-in function) proves it.

Review findings

• Input paths: only SyncRuleMappingSource.Expression is live today (Administrator-only via the REST API, the Attribute Flow tab, and New-JIMSyncRuleMapping). ObjectMatchingRuleSource.Expression and ExampleDataTemplateAttribute.Expression are model-ready but have no reachable authoring surface yet (verified: no DTO/UI editor/cmdlet populates them, and seeded templates carry no expressions); they inherit the evaluator's guardrails automatically if wired up later.
• No untrusted-input path: attribute values flow through the get-only mv/cs accessors as data, never as expression source; hostile-looking values are proven inert.
• What is blocked (proven by 28 security tests, not asserted): chained reflection (.GetType().GetMethods(), .Assembly, typeof(double).GetMethods()) throws ReflectionNotAllowedException; unregistered types (Process, FileStream, Environment, Activator, AppDomain) fail as unknown identifiers; assignment has no settable target. Validate rejects everything Evaluate rejects, so protection applies at save-time too.
• Open finding, accepted as documented (validator decision): a bare mv["a"].GetType() parses and evaluates, returning an inert System.Type (DynamicExpresso's reflection blocker rejects anything chained onto a Type, and Type.Name is its one documented allowed member). Chaining is blocked, the value grants no capability, the author is a trusted Administrator, and blocking it would need brittle custom parse inspection; documented in the review rather than hardened.

Guardrails added (both TDD, red first)

• MaxExpressionLength (10,000 characters) across Evaluate/Validate/Test: a paste-bomb sanity bound roughly two orders of magnitude above the longest documented real expression.
• MaxCompiledExpressionCacheSize (1,000 entries) on the previously unbounded static compiled-expression cache, which ad-hoc expression-tester usage grew forever; clear-and-log on reaching the bound (recompilation is sub-millisecond).

Rejected measures are recorded in the review with rationale: function/operator allowlists, removing default reference types, and evaluation timeouts all impair legitimate or future transformation capability for no matching risk reduction against a trusted-author threat model.

Validation

• dotnet build JIM.sln: 0 errors, 0 warnings. dotnet test JIM.sln: 3,436 passed (+80 new), 35 skipped (category-gated), 0 failed.
• TDD evidence: guardrail tests written first and failed for the right reason (missing constants) before implementation.
• DynamicExpresso claims verified against the pinned 2.19.3 source, then proven against the real evaluator by unit tests.

Note: this branch and #983 both move the OWASP assessment doc to plans/doing/ and touch the same COMPLIANCE_MAPPING row; whichever merges second will be brought up to date with main and reconciled before merge.

Docs: n/a - internal security review; the changelog entry is 🔒 and behaviour is unchanged for administrators.

🤖 Generated with Claude Code