fix(policy): evaluate every targeted f:query before denying; deterministic policy order

[bplatz]

Problem

When two or more targeted policies (f:onClass / f:onProperty / f:onSubject) apply to the same flake and are not f:required, the allow-overrides pass returned deny on the first targeted f:query that produced no rows, without evaluating the remaining targeted policies. The docs promise "any allow is enough" across the non-required set.

Which policy was tried first was random: both the same-ledger builder (policy_builder.rs) and the cross-ledger materializer (policy_materializer.rs) collected policy subjects into a HashSet and iterated it. Under cross-ledger policy the governance cache pinned one random order per model-ledger version, so a rule change appeared to "sometimes" take effect and then stayed wrong until the next model commit or a restart. Same-ledger, identical queries could flip between allow and deny with no commit in between.

Repro shape: two f:AccessPolicy rules with f:query on ex:Line, one matching identity A and one matching identity B. Before this change roughly half of the rebuilt policy sets denied one of the two identities.

Fix

• fluree-db-policy/src/evaluate.rs: in both async evaluators, every targeted entry is evaluated before deciding. A failing targeted f:query still blocks fall-through to Default policies. The detailed variant reports all failed targeted policies as deny candidates.
• policy_builder.rs and policy_materializer.rs: policy subjects are sorted before loading, so the policy set and the cross-ledger wire artifact are deterministic.
• Docs (policy-model.md, programmatic-policy.md) state that every targeted policy is evaluated regardless of load order.

Tests

• Unit: order independence for a fail-then-pass and pass-then-fail pair; a failing targeted query still blocks a Default allow.
• Integration: 16-round cross-ledger test with a fresh materialization per model commit, and a same-ledger twin with repeated identical queries. Both fail against the previous sources.

Behavior note

Sorting the policy subjects changes one user-visible thing:
PolicyDecision::deny_message() returns the first candidate carrying an
f:exMessage, so when several applicable policies each set one, which message a
user sees was previously downstream of HashSet iteration order and is now
Sid order. That moves random → deterministic, and on the targeted path the
candidates are now the policies that actually failed rather than every policy
that applied — but a caller who happened to be seeing one particular
f:exMessage may now see a different one.

Follow-up: #1787 (sync evaluator fails open in the same shape; unreachable today, but it now contradicts the guarantee this PR documents).

[aaj3f]

@bplatz good catch and good fix. Claude helped find a performance consequence but I'll just mention that myself before providing Claude's detailed review below:

---

Praise for the work — and the body's diagnosis is the real one: at BASE the non-required loop returned deny on the first targeted f:query with no rows, and the HashSet<Sid> iteration in both builders made which policy went first random per materialization, so under cross-ledger policy the governance cache froze one wrong answer per model version. I re-verified that rather than trusting it: the new order-independence unit test goes red the moment the old early return is put back (evaluate.rs:1163), the two integration tests run inside the declared grp_policy target (16 rebuilds × 2 identities same-ledger, and the cross-ledger materialization twin), and the direction of the defect was over-deny only — the fix grants nothing the docs didn't already promise. The detailed evaluator now naming every failed targeted policy as a deny candidate is a genuine diagnostics win too.

The one thing I'd fold in before merge is small: the partition at evaluate.rs:511 (and :766) allocates two Vecs per flake on the non-required path where two filtered passes over the existing filtered_entries would allocate nothing — snippet inline. Everything else is a question or praise.

Adherence to repo commitments:

• Patterns/abstractions: ✔ extends the existing evaluator and builders; no parallel construct; SPARQL/JSON-LD parity untouched (policy layer only).
• Performance (speed first, memory second): ⚠️ allow path unchanged; deny path now runs every targeted f:query on the target by design (the old early return was the bug); two avoidable per-flake Vec allocations — see the inline note.
• Testing: ✔ two unit tests + two integration tests in the declared grp_policy target; mutation-verified; CI nextest green on the head.
• Conventions: ✔ multi-line conventional commit with the mechanism; docs updated on both pages; clippy -D warnings green in CI.

Verified locally at branch HEAD 75b6d3574: cargo test -p fluree-db-policy for both new unit tests → ok; cargo test -p fluree-db-api --test grp_policy multiple_targeted_query_rules → 2 passed; mutation (BASE early return restored) → targeted_query_policies_are_order_independent FAILED as expected, then restored clean; BASE vs HEAD read of both evaluators and both builders.

Approving so you can merge when ready, but maybe worth considering the allocation note first.

[aaj3f]

Optional (performance, fold in now). I don't think this is the expensive part of the path — the f:query evaluations dwarf it — but filtered_entries.into_iter().partition(..) here (and the twin at :766-770) allocates two Vecs per flake evaluation on the non-required path, where the previous code allocated nothing further after filtered_entries itself.

Since filtered_entries is already a Vec, the same walk falls out of two filtered passes with no allocation:

let is_targeted = |e: &FlakePolicyEntry| policy_set.restrictions[e.idx].target_mode != TargetMode::Default;
let mut targeted_query_failed = false;
for entry in filtered_entries.iter().filter(|e| is_targeted(e)).chain(filtered_entries.iter().filter(|e| !is_targeted(e))) {
    // unchanged body; the `!is_targeted && targeted_query_failed` break still applies
}

Per-flake allocation on the policy path is the kind of thing we grade hard on, so minor and non-blocking — but if you agree it's right, I'd rather see it folded in now than lost in the backlog.

[bplatz]

Folded in at 1efca42 — both sites (:511 and the detailed twin) now walk filtered_entries twice with a filter instead of partitioning into two Vecs. Agreed the f:query evaluations dwarf it, but it is a per-flake allocation on the policy path, so better gone.

The break is preserved as-is (your other note): the second filtered pass yields only Default entries, so they still all follow the targeted ones and !is_targeted && targeted_query_failed stops at the first Default exactly as before. targeted_query_policies_are_order_independent and failing_targeted_query_blocks_default_fallthrough both still pass, along with the full grp_policy target (98 passed) on the branch after merging current main.

One thing your note made me notice, not worth changing here: the detailed variant already allocates candidate_restrictions unconditionally at :708, including on the allow path. So this removes the two allocations the PR added but does not make that evaluator allocation-free.

[aaj3f]

Praise. The !is_targeted && targeted_query_failed break is the load-bearing line — it's what keeps "a failing targeted query still blocks fall-through to Defaults" true after the early return goes away, and failing_targeted_query_blocks_default_fallthrough pins exactly it. Worth preserving as-is if the loop gets restructured.

[aaj3f]

Praise, and one question. Sorting the subjects is the right determinism fix (the wire artifact and the governance cache both benefit).

This is more of a question than a suggestion: is there anywhere else a policy set's order is user-observable — the explain/diagnostics output listing candidates, say — where the switch from HashSet order to Sid order would change what people have been seeing? If not, nothing to do.

[bplatz]

Good question, and yes — there is exactly one place, PolicyDecision::deny_message() (fluree-db-policy/src/types.rs:249). It returns the first candidate that carries an f:exMessage:

PolicyDecision::Denied { candidates } => candidates
    .iter()
    .filter_map(|r| r.message.as_deref())
    .next(),

So when several applicable policies each set f:exMessage, which one a user saw was previously downstream of HashSet<Sid> iteration order and is now Sid order. That is user-visible, but it moves in the right direction on both axes:

• random → deterministic. The same denial stops showing a different message run to run (and, under cross-ledger policy, stops freezing one arbitrary message per model-ledger version in the governance cache).
• more accurate. On the targeted path this PR narrows candidates from candidate_restrictions to failed_targeted, so the message now comes from a policy that actually failed rather than from an arbitrary policy that merely applied.

Nothing else is order-sensitive that I could find: the Allowed arm returns the specific winning restriction rather than an ordinal, the required path reports candidate_restrictions.first() only after every gate has granted (so the set is fixed, not the order), and the tracker records executions as a set of ids. load_policies_of_classes is the single same-ledger funnel — load_policies_by_identity hands it a Vec, not a set — with materialize_policy_rules as the cross-ledger twin, so the two sorts cover it.

The one caveat worth stating plainly: anyone who happened to be relying on a particular f:exMessage surfacing may now see a different one. I have added a line to the PR body rather than changing code, since the previous behavior was not a guarantee anyone could have depended on deliberately.