report policy enforcement state in tracked query meta - #1627

"on any endpoint" overclaims. /v1/fluree/stream/query/*ledger does enforce policy in its ledger-scoped form, but execute_view_streaming (fluree-db-api/src/view/stream_query.rs:215) never calls record_policy_enforcement, and stream_tracker forces fuel/time on while the end record carries only those two.

So on the endpoint most likely to be used for large policy-filtered result sets, the signal is unavailable — while this same page tells the reader that absent means unenforced. That's a false negative on exactly the question the page exists to answer.

Suggest scoping the sentence to /query + /query/sparql now and filing the streaming wiring as a follow-up.

You're right, and thank you for chasing it into stream_query.rs — this was the best catch in the review. The sentence was writing a check the streaming surface couldn't cash, and the failure mode is the bad one: the page tells the reader that absence means unenforced, so a caller streaming a policy-filtered result would have read silence as "nothing filtered me". A false negative on the one question the page exists to answer.

I went with wiring it rather than scoping the prose. execute_view_streaming (fluree-db-api/src/view/stream_query.rs:215) now calls record_policy_enforcement before it prepares, exactly as its buffered sibling execute_view_tracked_with_r2rml does, and I put the dataset-side call in execute_dataset_into_with_r2rml (fluree-db-api/src/view/dataset_query.rs:658) since that's the sink-generic core the streaming dataset producer and the buffered "into" path already share — so the two can't diverge in setup later. The terminal end record then carries policy_enforcement under the same presence rule the buffered response uses, so a client can read absence identically on both surfaces.

On the protocol blast radius, which was the thing I wanted to be sure of before extending the record: end already carries t/fuel/time conditionally, so an optional field is the shape clients are built for, and nothing in the workspace deserializes these records into a deny_unknown_fields struct. Our own consumer (fluree-db-cli/src/commands/query_stream.rs:123) classifies on type and pulls fields individually with .get(), and in envelope mode passes every record through verbatim — so an older CLI against a newer server just ignores it. I added a consumer test for the older-server direction too (field absent ⇒ None, no invention).

Two things worth flagging that I did not change. The state is settled before the first row is pulled but only reported at the end — putting it on head would get it to the caller earlier, but head is emitted before execution begins and I'd rather not have two sources of truth for it; happy to revisit if you think early notification matters more than I'm weighting it. And policy (the per-policy counters) still isn't carried on the streaming surface — that one is data-shaped (it counts evaluations), so I left it off deliberately rather than by omission.

Tests: fluree-db-server/tests/stream_query_integration.rs now pins all four states over the real endpoint — enforced-with-grants, the opt-in off case, zero-policy deny-all, and anonymous-absent.

Small precision point: "a pure function of the request's policy inputs and the ledger's policy configuration" understates what's involved — the view set includes stored policies selected through the identity's f:policyClass assignments, which are ledger data.

The oracle argument still holds (constant per caller/ledger, query-independent, and you have to be the identity to see it), so this is a wording fix rather than a design problem. The sentence should just say so rather than implying no ledger data participates.

You're right and I'm glad you pushed on the phrasing — "a pure function of the request's policy inputs and the ledger's policy configuration" was doing something I didn't intend, which is implying no ledger data participates at all. It plainly does: the view set is assembled from stored f:AccessPolicy nodes, and which ones land in it is decided by the identity's own f:policyClass assignments, which are flakes like any other.

Rewritten in fffad550e (pre-rebase SHA; the branch has since rebased onto main — head 83edb9d50 carries the same change) to say that outright and then rest the argument on the three properties that actually carry it — the value is settled when the view is built and doesn't depend on the data the query reads or on the query itself; it's constant for a given caller and ledger and moves only when the policy config or that caller's own assignments move; and you have to already be acting as the identity to see it, so what it discloses is your own authorization posture. I propagated the same correction to GraphDb::policy_enforcement and to the three docs pages that had inherited the loose phrasing, since it would have been odd to fix it in one place and leave the docs overclaiming.

I traced all four orderings — [unenforced, enforced-deny], [enforced-deny, unenforced], [deny, deny], [grant, deny] — and this produces the right answer in every one, so no correctness objection.

But it's hard to verify by reading: is_none_or(|a| a.denies_all_data) && state.denies_all_data combined with a None arm that seeds PolicyEnforcement::default(), whose enforced: false is then load-bearing for the trailing .filter, is a lot of implicit state for two booleans. A fold over (any_enforced, all_enforced_deny) would be a third the length and self-evident.

More actionable: this function has no unit test. The tracker's merge has one, but the dataset aggregation — where the ordering subtlety actually lives — doesn't.

Fair on both counts, and the "no unit test" half is the one that stung — you're right that the tracker's merge has coverage and the place where the ordering subtlety actually lives had none. Thanks for tracing the four orderings by hand; that's exactly the work the test should have been saving you.

Refactored in c50c5769a (pre-rebase SHA; the branch has since rebased onto main — head 83edb9d50 carries the same change) to the fold you sketched, over (any_enforced, all_deny) — any is an OR and all an AND over one pass, so the order-independence is now evident from the shape instead of needing a trace. I pulled it out as a free function aggregate_policy_enforcement taking impl IntoIterator<Item = Option<PolicyEnforcement>> rather than leaving it inline, mostly so the test can exercise the aggregation directly without standing up ledgers and policy wrappers to manufacture each ordering — the logic under test is the fold, and going through real GraphDbs would have made the test mostly fixture.

The test covers your four orderings plus empty and all-unenforced. The case I most wanted pinned is the one you called out: an unenforced graph has to clear the whole-dataset deny bit without clearing enforced, whichever side of an enforced graph it falls on, since one readable graph is enough for the request to return data.

This flips the response shape for a request sending only fluree-track-policy: bare result array -> tracked envelope. Nobody could have depended on the tracked shape (the header was never parsed), but clients sending the header and parsing a bare array will break — including any CLI older than this build talking to a newer server. Worth a release note.

Good catch on who actually breaks here — I'd been thinking about it as "nobody could have depended on the tracked shape" and stopped there, which is true but is the wrong half of the population. The people at risk are the ones sending the header and parsing a bare array, and as you say that includes any CLI older than this build pointed at a newer server, where --track-policy has been a silent no-op all along.

Added to docs/reference/compatibility.md under a new "API Versioning → Behavior changes" section (9aa2e52b4 (pre-rebase SHA; the branch has since rebased onto main — head 83edb9d50 carries the same change)), covering what flips, who is affected, and the two ways to adapt — read result when the body is an object and the body itself when it's an array (which is what our CLI does, and works against either server), or stop sending the header. I documented the additive policy_enforcement field there too, including the streaming end-record form, so there's one place to look. Let me know if you'd rather this also ride the release notes proper when we cut the version — happy to add it there, I just didn't want the compat page and the notes to drift.

Correct for CI (--all-features), but it makes the file invisible under a plain cargo test -p fluree-db-cli. Given the test exists precisely to catch drift, worth a line in the header saying so — otherwise someone renames a header locally, sees green, and ships it.

I may be misreading which invocation you had in mind, but server is a default feature of fluree-db-cli (fluree-db-cli/Cargo.toml:23, default = ["server", "iceberg", "shacl", "aws"]), so a plain cargo test -p fluree-db-cli does compile and run this file — I re-ran it just now to be sure and it reports 3 tests. So the drift scenario you describe (someone renames a header locally, sees green, ships it) shouldn't happen on a default-feature run.

Where you're right, and it's the same hazard: under --no-default-features, or any feature set that drops server, the file vanishes as a silently empty test binary — a crate-level cfg leaves nothing behind to report as skipped, so it looks like a pass. That's the vacuous-check failure mode we've been bitten by before, so it earns the comment either way.

So I've written the header note (9aa2e52b4 (pre-rebase SHA; the branch has since rebased onto main — head 83edb9d50 carries the same change)) to say precisely when it runs and when it disappears, and to tell anyone testing with a narrowed feature set to confirm these test names appear in the output before concluding the seam is intact. If you did mean --no-default-features all along then we're just agreeing and I've belabored it — apologies if so.

This is the right call, and well documented — deliberately not adding the field so postcard offsets don't shift, reasoning in the comment, plus a round-trip test asserting None. Nothing to change.

Thank you — and thanks for reading the postcard reasoning closely enough to confirm it, since that's the sort of thing that's easy to skim past as boilerplate. Leaving it exactly as is.