Implement Stage A: rule engine, validation, audit, and safety gates
Completes 109 of 121 tasks. Every remaining task needs a tenant connection
(T055, T056, T101-T103) or an Azure Automation account (T115-T121).
354 offline Pester tests PASS
Engine purity (Principle IV) PASS
Sanitization (SC-013) PASS (156 files)
Graph module loaded in tests none (SC-008 holds)
What landed
- Four-layer configuration validation with stable finding codes, covering
every VR-002 and VR-003 condition, plus a 23-fixture invalid-config corpus
- Run loop, audit records (NDJSON through a single sink), summaries,
reconciliation, and exit codes 0-6
- Persistence behind a single write-body builder whose result always has
exactly one key
- Invoke-PersonaEngine.ps1 and Edit-PersonaEngineConfig.ps1
- Six docs, two pipelines, traceability matrix, V-5a and sanitization records
Three deviations from tasks.md, each recorded in its status block
T033 is not in Resolve-UserPersona. evaluationErrorThreshold is run-level
state and the rule engine is pure; a counter there would break Principle IV.
It lives in New-PersonaRunCounter and is applied in the run loop.
A new src/Engine/ layer holds Invoke-PersonaEngineRun. The entry script
imports the manifest, which requires Microsoft.Graph.Authentication, so a
loop living only inside it could not run on a machine without the Graph SDK
and SC-004 could not be proven at all. The entry script is now a thin
wrapper and what ships is what is tested.
The invalid-config corpus is generated by a committed script, with the
generated fixtures committed too, so a reviewer sees the fixture in the diff.
Defects found by running the code, not by reading it
Group and role ID lists were double-wrapped: @(Get-PersonaGroupIdPage ...)
around a comma-returned array collapsed every membership list into one
bogus space-joined entry. That is a silent false non-match, exactly what
FR-013 exists to prevent.
A 403 whose status appears only in the exception message parsed as $null,
which the retry policy treats as a transport error - five requests per
account against a tenant already refusing. Status extraction now falls back
to the message text, bounded to 400-599.
The sanitization scan walked tracked files only, so it covered 34 of 156
files and none of this phase's code. It now scans untracked non-ignored
files too, and a negative control confirms it catches a planted leak.
Test-Json reports one error per violating location, not first-failure-only
as the V-5a draft claimed. Record and pin corrected.
Enforcement remains blocked on the V-4 security sign-off.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,137 @@
|
||||
# Business rules
|
||||
|
||||
How to write, order, and change the rules that decide what an account is.
|
||||
|
||||
Field-by-field syntax is in [ConfigurationReference.md](ConfigurationReference.md). This document is
|
||||
about judgement.
|
||||
|
||||
## The model
|
||||
|
||||
Rules are evaluated in ascending `priority`. The first rule that returns `True` wins, and evaluation
|
||||
stops. An account matching no enabled rule is `Unclassified`. An account whose evaluation hits data
|
||||
that could not be retrieved is `EvaluationError`, and its stored value is preserved.
|
||||
|
||||
Three consequences worth internalising before writing a rule:
|
||||
|
||||
**Order is meaning.** A rule at priority 900 only ever sees accounts that failed every rule above it.
|
||||
Changing a priority silently reclassifies every account matched by more than one rule, which is why
|
||||
`PE-SAF-005` blocks a reorder without a `configVersion` change.
|
||||
|
||||
**A rule cannot express "and not the previous ones".** It does not need to. First-match already
|
||||
excludes them. Adding explicit exclusions duplicates the ordering in two places, and the day they
|
||||
disagree the ordering wins silently.
|
||||
|
||||
**Determinism is absolute.** The same account and the same configuration always produce the same
|
||||
persona. Nothing time-dependent, random, or order-dependent may enter a decision — `effectiveDate` is
|
||||
metadata for exactly this reason, and the engine's per-user timing uses a monotonic stopwatch rather
|
||||
than the wall clock so no clock value can reach a decision.
|
||||
|
||||
## Priority bands
|
||||
|
||||
A convention, not enforced, but it makes the intent of a rule set legible at a glance:
|
||||
|
||||
| Band | Purpose | Examples |
|
||||
| --- | --- | --- |
|
||||
| 1–99 | Accounts that must never be reclassified by anything | Emergency access, Tier 0 |
|
||||
| 100–199 | Directory facts that are definitional | Guest, external |
|
||||
| 200–499 | Non-human accounts | Service, shared functional, room devices |
|
||||
| 500–799 | Population subsets | Contractor, student, restricted |
|
||||
| 800–999 | Defaults | Employee |
|
||||
|
||||
Leave gaps. Renumbering to insert a rule is a reorder, and a reorder is a `PE-SAF-005` finding.
|
||||
|
||||
## Writing a rule that holds up
|
||||
|
||||
**Identify special accounts by Object ID, never by name.** Display names and UPNs change; Object IDs
|
||||
do not. RE-009 exists because a break-glass account renamed during an incident must not silently stop
|
||||
being a break-glass account.
|
||||
|
||||
**Require two independent signals for a consequential classification.** The example configuration's
|
||||
service-account rule requires both a naming convention *and* group membership, so a person whose UPN
|
||||
happens to start with `svc-` is not classified as a service account.
|
||||
|
||||
**Prefer group membership to string matching for anything privileged.** A group is administered,
|
||||
auditable, and has an owner. A naming convention is a habit.
|
||||
|
||||
**Give every rule a description that says why it exists**, not what it does — the conditions already
|
||||
say what it does. A rule nobody can explain cannot be safely changed, which is why `description` is
|
||||
required.
|
||||
|
||||
## Membership mode
|
||||
|
||||
`direct` asks whether the account is a member of the named group itself. `transitive` asks whether it
|
||||
is a member through any chain of nesting.
|
||||
|
||||
Mode is a per-condition choice (RE-007). The three facets — direct, transitive, roles — are retrieved
|
||||
independently, so mixing modes in one rule set is fully supported. It costs one extra request per
|
||||
account for each additional facet.
|
||||
|
||||
Use `transitive` when the group is a role-holding group that other groups nest into — which is most
|
||||
Tier 0 groups. Use `direct` when membership is explicitly managed and nesting would be a mistake.
|
||||
|
||||
**Do not pin `dataSources.groups.membershipMode` unless you mean to restrict.** Absent means "any mode
|
||||
is acceptable". Pinning it turns every per-condition override into a `PE-SEM-014` warning, which
|
||||
trains people to ignore warnings.
|
||||
|
||||
## Unknown is not false
|
||||
|
||||
If a membership lookup fails, the condition is `Unknown`, not `False`. A `notMemberOf` condition
|
||||
therefore does **not** become satisfied when the lookup fails.
|
||||
|
||||
This is the single most important behaviour in the engine. Without it, a transient Graph outage would
|
||||
make every privileged account look like a non-member of its Tier 0 group, and a single run would
|
||||
quietly demote the entire administrative population. `UnknownNotFalse.Tests.ps1` exists solely to
|
||||
prevent that regression.
|
||||
|
||||
The cost is that a failed lookup produces `EvaluationError` rather than a classification. That is the
|
||||
correct trade: preserving a possibly-stale value is recoverable, and writing a confidently wrong one
|
||||
is not.
|
||||
|
||||
## Nesting
|
||||
|
||||
`all` and `any` groups nest to `maxConditionDepth` (default 5, ceiling 10). Beyond the limit the
|
||||
engine returns `Unknown`, which becomes `EvaluationError` for every account the rule reaches — so a
|
||||
too-deep rule fails safe rather than silently.
|
||||
|
||||
A rule needing more than three levels is usually two rules with different priorities. Depth is
|
||||
expensive to read and the ordering you would express with nesting is already available for free.
|
||||
|
||||
## Changing a rule set
|
||||
|
||||
1. Edit the configuration.
|
||||
2. Validate against the deployed copy so the drift checks actually run:
|
||||
|
||||
```bash
|
||||
pwsh ./Edit-PersonaEngineConfig.ps1 -ConfigPath ./config/persona-engine.json -PreviousConfigPath ./deployed/persona-engine.json -ValidateOnly -NonInteractive
|
||||
```
|
||||
|
||||
3. Run the rules against synthetic fixtures.
|
||||
4. Preview against one account, then against the tenant with `-WhatIf`.
|
||||
5. Compare the summary's per-rule match counts against the previous run. A rule whose count moved
|
||||
sharply is either the change you made or a change you did not intend.
|
||||
6. Raise `configVersion`.
|
||||
|
||||
Step 5 is the one people skip. The summary lists every rule including zero-match ones precisely so
|
||||
that a rule which *stopped* firing is visible, and a rule that stopped firing is the usual signature
|
||||
of an accidental reorder.
|
||||
|
||||
## Disabling versus deleting
|
||||
|
||||
Disable rather than delete. A disabled rule still appears in every summary with a zero count, so the
|
||||
audit trail keeps reporting on it and an operator can see it was deliberately turned off. A deleted
|
||||
rule is indistinguishable from one that never existed, which is why `PE-SAF-005` flags a deletion
|
||||
without a version change.
|
||||
|
||||
## Testing a rule set without a tenant
|
||||
|
||||
```bash
|
||||
pwsh ./Edit-PersonaEngineConfig.ps1 -ConfigPath ./config/persona-engine.json -TestDataPath ./tests/TestData -ValidateOnly -NonInteractive
|
||||
```
|
||||
|
||||
This runs the real engine against the synthetic fixtures in `tests/TestData/`. The fixtures include
|
||||
accounts with null and absent properties, mixed casing, a guest, a disabled account, and — most
|
||||
usefully — two accounts whose membership lookups failed, so `EvaluationError` behaviour is visible
|
||||
before it happens against a real directory.
|
||||
|
||||
Add fixtures for the cases your rule set actually cares about. A fixture that reproduces a real edge
|
||||
case, sanitized, is worth more than any amount of reasoning about what the engine will probably do.
|
||||
Reference in New Issue
Block a user