feat(policy): enforce static view policy on Iceberg/SQL graph sources - #1759
Conversation
View policy never reached an R2RML scan: the operator emits string-IRI bindings with no index behind them, so every policy shape fell open — onProperty/onClass/onSubject denies, default-allow false, even an unknown identity all returned every row, while the same policy on a native ledger returned none. The scan now carries a lane-local policy gate (`r2rml::policy`). It re-indexes the request's view restrictions against class→property stats derived from the mapping (a virtual source has no index stats) and selects class policies by the row's classes, then evaluates each read triple with the engine's own evaluator. Subject classes come from `rr:class` and constant `rdf:type` maps, or are materialized per row when a map derives `rdf:type` from a column (the type columns are added to the projection). Decisions are memoized per (triples map, predicate) unless a policy targets subjects, and a map whose required predicates are all hidden is skipped before its table is read. `f:query` policies evaluate as "no rows" and deny their targets: there is no graph to run them against. The fused aggregate fold declines under a policy so counts go through the gate. Stored policies and the class hierarchy need somewhere to live, so a source can name a model ledger (`--model`, HTTP/config `model`) whose default graph becomes its `f:policySource` and `f:schemaSource` through the existing cross-ledger resolver. A bare identity is looked up in the model ledger for its `f:policyClass`, and the schema-bundle translation no longer drops axioms whose namespaces the (genesis) data snapshot never registered. `it_iceberg_policy` checks every static shape and pattern form against a native twin of the same fixture as the oracle, plus fail-closed `f:query`, column-derived classes, dataset mode, and the model-ledger flow.
…del, and cross-ledger pages
…models A virtual source silently denied the targets of any f:query policy, and a mistyped --model surfaced only as a 502 on every governed query. Now: - the tracked response's policy_enforcement lists unevaluable_policies (the f:query ids the source could not evaluate), the server logs a warning the first time each is met, and skipped triples maps log at debug; - --model is validated at registration (must be an existing native ledger) and policies in it that use f:query are reported as model_warnings in the CLI output and HTTP response; `fluree model access enable --connected` warns when a registered source is governed by the model; - sources take --default-allow / default_allow so an admin can keep a model-less source readable under authentication, the counterpart of a native ledger's f:defaultAllow; - ledger-info's source block reports model and default-allow.
# Conflicts: # docs/graph-sources/sql.md
Review follow-ups on the static graph-source policy work. Each fix has a regression test with a control assertion, verified non-vacuous by reverting the fix and watching the test fail. Scan-side top-k must be declined under a view policy. The policy gate drops rows after the scan emits, so a denied row can set the k-th bound and prune files whose visible rows belong in the true top-k. On the two-file test fixture, an f:onSubject deny on the top scorer with LIMIT 1 answered with the lowest-scoring row instead of the highest visible one — a wrong row, not just a short result. `resolve_topk_directive` now declines alongside the existing residual-filter check. The graph-scoped builder (`fluree.graph(id).query()`, which the server's proxy-mode query route uses) never wrapped policy, so a --model-governed source was read unfiltered where the from-driven builder correctly denied. Attaching the model's `resolved_config` alone did nothing because nothing downstream consumed it. Wrap on that path, gated on the request carrying a policy input — the same rule `apply_source_or_global_policy` applies, so a request with no policy inputs stays unrestricted as for a native ledger. A failed graph-source lookup now propagates instead of silently reading as "ungoverned". A graph source's system graphs (#txn-meta) are genuinely empty and are deliberately not tagged as the virtual source, but `resolved_config` was assigned before the graph-ref match and rode along. `wrap_policy` then took the cross-ledger path with virtual_source = false and rejected an identity carrying no explicit f:policyClass, turning an empty result into a config error. Moved into the GraphRef::Default arm to sit with `graph_source_id`. Note: the graph-scoped builder still ignores opts.policy for native ledgers. That is pre-existing and unchanged here.
Follow-ups from the same review pass. None of these had a reproducible leak; each is a consistency or diagnosability fix, so no behavior change is claimed and no regression test is added. `required_predicates` keyed the bare-subject `rdf:type` fallback on `tm.classes()` while the gate's `tm_classes` keys on `static_classes`, which also folds in constant `rdf:type` predicate-object maps. Aligned on the wider set; it can only add a required predicate, never drop one. The shape that would expose the difference (a subject-only pattern binding no predicate variable) was not constructible through the query surfaces. Reaching the enforcer lookup with no enforcer in scope contradicts `allow_unfiltered`, which already said filtering is required. That state was not reachable — probing every graph-source and policy suite, plus a dataset carrying a per-source policy and no global one, never produced it, because `with_graph_ref` supplies the graph's enforcer and clears `dataset` before a scan opens. Left the behavior alone and asserted the invariant so a future path that opens a scan on a dataset-level context fails in CI rather than scanning unfiltered. The once-per-process `f:query` warning deduped on the policy id while the message names the graph source too, so a second source governed by the same model never logged. Keyed on both.
The wasm32 clippy lane builds `fluree-db-api` with `--no-default-features`, where both callers — graph-source registration for Iceberg and for SQL — are compiled out, so the method linted as dead and failed the gate. Gated on the same features. Reproducible on the host with `cargo clippy -p fluree-db-api --no-default-features`.
aaj3f
left a comment
There was a problem hiding this comment.
This is a genuinely important fix, @bplatz, and we talked about it a bit in-office. No disagreements with design or with decisions (i.e. how to narrow the scope of possibilities for what can be policy-enforced reasonably on remote Iceberg/SQL data). I'll provide Claude's review below:
Praise for the following: — the same allow_view_flake_async the native path uses, running as a lane-local gate inside the R2RML scan with the view restrictions re-indexed against mapping-derived stats, rather than a second policy implementation for virtual sources. I verified it rather than reading it: with --features iceberg,native all 13 it_iceberg_policy tests pass on this head, and forcing the gate to always-allow makes static_policies_match_native_twin fail at :197 with the native twin denying what the virtual source returned — so the parity oracle has teeth. f:query failing closed and landing on the tracked response as unevaluable_policies (plus the once-per-source warning) is the right way to make the one thing a virtual source can't do visible instead of silent, and catching the proxy-mode builder in commit 3 — where a governed source would have been read unfiltered — is the kind of entry-point audit that usually gets missed.
Two things I'd fold in now. First, a performance nit on the gate: allows allocates the (tm.iri, pred) memo key as two Strings per row per predicate before checking the cache, and on the static path the per-row loop is redundant anyway (same answer for every row in the window) — hoisting the static decision out of the row loop makes the common case allocation-free. Second, the pre-existing gap your commit body already names — the graph-scoped builder ignores opts.policy for native ledgers, which I confirmed — is the same class of bypass this PR closes for virtual sources, so it deserves a scoped fix soon; I'm not asking for it here because whether proxy mode should wrap policy for native ledgers is a scope call, not a review thread. Also worth a line in the title or first paragraph: the fail-closed behavior for model-less sources under authentication is a visible upgrade change and should reach the release notes.
Adherence to repo commitments:
- Patterns/abstractions: ✔ reuses
PolicyContext/allow_view_flake_asyncand the cross-ledgerf:policySource/f:schemaSourceresolver; no parallel evaluator; SPARQL/JSON-LD parity untouched (scan-level). - Performance (speed first, memory second): ✔ no-policy path byte-for-byte unchanged (
policy_gate: None); under a policy: memoized per (map, predicate), fully hidden maps skipped before their table is read;⚠️ two avoidableStringallocations per row on the static path — see the inline note; fold and top-k decline under policy is correctness over speed and documented. - Testing: ✔ 13 integration tests against a native-twin oracle in a declared
[[test]](required-features = ["iceberg","native"], so they run under CI's--all-features); mutation-verified; three regression tests in commit 3 the author verified red-then-green. - Conventions: ✔ thorough multi-line commits; docs across six pages; wasm32
--no-default-featuresclippy fixed in-branch; conventional titles.
Verified locally at branch HEAD c221efdd0: cargo test -p fluree-db-api --features iceberg,native --test it_iceberg_policy → 13/13; cargo test -p fluree-db-core tracking → green; mutation (gate → always allow) → parity test FAILED as expected, restored clean; entry-point audit of server routes, SQL bridge, Cypher, and the two query builders; Cargo.toml [[test]] wiring confirmed.
Approving so you can merge when ready — maybe worth the allocation hoist first, and let's get the native-ledger builder gap on the board.
| row_classes: &[String], | ||
| ) -> Result<bool> { | ||
| let static_decision = !self.per_subject && row_classes.is_empty(); | ||
| let key = (tm.iri.clone(), pred.to_string()); |
There was a problem hiding this comment.
Should-fix (performance, fold in now). allows builds the memo key (tm.iri.clone(), pred.to_string()) before it checks static_cache, so on the static path every row pays two String allocations per required predicate even on a cache hit.
And on that path the answer is the same for every row in the window, so the per-row loop itself is redundant: filter_rows (:179-224) already knows required and whether the decision is static (!self.per_subject && row_classes.is_none()).
I'd decide each required predicate once per call before the row loop and only walk rows for the predicate_var and per-subject cases; the cache can then be keyed on borrowed (&str, &str) or on required's index.
It isn't a regression — nothing enforced anything here before — but it's a per-row allocation in the scan operator, which is the thing we grade hardest on, and the fix is local. If you agree it's right I'd rather see it here than in the backlog.
There was a problem hiding this comment.
Agreed, and folded in as 37a0e7a.
You were right that the per-row loop is the bigger half. On the static path — no subject-targeted restriction, no column-derived classes — every input to the decision is fixed for the whole window, so filter_rows now settles required and the projected rdf:type once before it walks anything. A denial returns the empty window; with no predicate variable in the pattern the rows pass straight through. Only a bound predicate variable, a subject-targeted policy, or column-derived classes put the gate back on the per-row path, and that path no longer copies the row's bound predicate into a String either.
static_cache is now nested (map IRI → predicate IRI → allowed) so a probe borrows both keys and only a miss allocates.
Behavior is unchanged by construction: passing no subject on the hoisted path is exactly what allows already substituted for a non-subject-targeted set, and row_classes.is_none() is precisely the condition under which every row previously took the static branch.
Verified the lane is not vacuous the way you did the gate: forcing the hoisted branch to return its window unfiltered makes wildcard_scan_hides_denied_predicates fail, then restores clean. All 13 it_iceberg_policy tests pass, as does the full fluree-db-query suite. Worth noting for the record that the two native-twin parity tests stay green under that mutation — their denials are caught earlier by the map-level tm_can_yield skip, so the wildcard test is the one pinning filter_rows' static deny path.
| crate::GovernanceOptions::from_json(json) | ||
| .map_err(|e| ApiError::query(e.to_string()))? | ||
| } | ||
| _ => crate::GovernanceOptions::default(), |
There was a problem hiding this comment.
Note (pre-existing, author-acknowledged; needs a scope decision). This hunk is right: a governed source through the graph-scoped builder now wraps policy when the request carries policy inputs, and a failed lookup propagates instead of reading as "ungoverned".
The commit body also says the graph-scoped builder still ignores opts.policy for native ledgers, and I confirmed it: load_view → load_graph_db_at with no wrap, and neither query_with_options nor query_tracked_with_r2rml_options applies policy from the input. So a native ledger read through fluree.graph(id).query() — the server's proxy-mode route at routes/query.rs:2234 / :2276 — with opts.identity set is unfiltered.
Not introduced here, and this PR narrows rather than widens it, but it's the same class of bypass this PR fixes for virtual sources, so it deserves its own fix soon. I'm not asking for it in this PR only because whether that route should wrap policy for native ledgers (or whether proxy mode is ever policy-bearing) is a call above a review thread.
There was a problem hiding this comment.
Already on the board: #1766, filed before this review landed, labeled bug / P1 / area:query. Your read of the path matches what's in the issue — load_view hands back load_graph_db_at's result unwrapped for native ledgers, and neither of the two execute methods applies policy from the input, so the proxy-mode JSON-LD route reads unfiltered with an identity set. Proxy mode being a real deployment shape (peer role plus proxy storage access) is why it's P1 rather than lower.
I've added it to this PR's "Not in this PR" section as Follow-up: #1766 so the deferral direction is explicit rather than inferred, per the repo's linking convention. Agreed it's a scope call and not a review thread — leaving it for its own PR.
| /// `rdfs:domain`, `rdfs:range`) bypass policy, so a query over the ontology | ||
| /// can still produce rows. | ||
| pub denies_all_data: bool, | ||
| /// Policies whose `f:query` could not be evaluated for this request and |
There was a problem hiding this comment.
Praise. unevaluable_policies with #[serde(default, skip_serializing_if = "Vec::is_empty")] keeps the wire shape additive for every consumer that already parses policy_enforcement (solo's contracts included), and the dataset merge deduping by id (:389-401) means a source governed twice by the same model reports each policy once. Nice.
| if topk_residual_filter_present(&self.pattern) { | ||
| return None; | ||
| } | ||
| // The view-policy gate is a residual filter too: it drops rows AFTER the |
There was a problem hiding this comment.
Praise. Treating the gate as a residual filter for the scan-side prune — and, in commit 3, declining top-k under it because a denied row can set the k-th bound and prune files whose visible rows belong in the true answer — is the correctness-preserving-fallback pattern done right. Worth keeping the comment.
On the static path — no view restriction targeting subjects, no column-derived row classes — every input to a policy decision is fixed for the whole materialized window: `allows` substitutes the placeholder subject, and the class set is the triples map's own. The gate nonetheless re-asked the question for each row and each required predicate, and built its memo key as two owned `String`s before probing the cache, so a scan under policy paid a pair of allocations plus a hash lookup per row per predicate on a window that can reach hundreds of thousands of rows. `filter_rows` now decides `required` and the projected `rdf:type` once before walking anything. A denial returns the empty window; with no predicate variable in the pattern the rows pass through untouched. Only a bound predicate variable, a subject-targeted policy, or column-derived classes put the gate back on the per-row path, and that path no longer copies the row's bound predicate into a `String` to ask about it. `static_cache` becomes a nested map so a probe borrows both keys and only a miss allocates. Behavior is unchanged: passing no subject on the hoisted path is what `allows` already did for a non-subject-targeted set, and `row_classes.is_none()` is exactly the condition under which every row previously took the static branch. The 13 `it_iceberg_policy` tests pass; forcing the hoisted lane to return its window unfiltered makes `wildcard_scan_hides_denied_predicates` fail, so the lane carries real enforcement rather than passing vacuously.
Summary
View policy never reached an Iceberg or SQL graph source. The R2RML scan emits string-IRI bindings with no index behind them, so the flake filter never ran and every policy shape fell open:
f:onProperty/f:onClass/f:onSubjectdenies,default-allow: false, even an unknown identity returned every row, while the same policy on a native ledger returned none.This PR enforces static targeting on virtual sources with native parity, gives a source somewhere to keep its policies and class hierarchy, and makes the one thing a virtual source cannot do (
f:query) visible instead of silent.Enforcement (
fluree-db-query/src/r2rml/policy.rs)The scan operator builds a lane-local policy gate when a non-root policy is active. It re-indexes the request's view restrictions against class→property stats derived from the mapping (a virtual source has no index stats) and also selects class policies by the row's classes, then evaluates each read triple with the engine's own evaluator.
rr:classand constantrdf:typemaps, or are materialized per row when a map derivesrdf:typefrom a column (the type columns are added to the projection).f:querypolicies evaluate as "no rows" and deny their targets. There is no graph to run them against.Covered shapes: fixed predicates, same-subject stars, constant objects, class scans, projected
rdf:type, wildcards, aggregates, andGRAPHblocks in dataset mode.Model ledger for virtual sources
A source can name a model ledger (
--model, HTTP/configmodel) whose default graph becomes itsf:policySourceandf:schemaSourcethrough the existing cross-ledger resolver. That supplies stored policies,rdfs:subClassOf/rdfs:subPropertyOfexpansion of targets, and identityf:policyClasslookup (done in the model ledger for virtual sources, which have no ledger of their own). The schema-bundle translation no longer drops axioms whose namespaces the genesis data snapshot never registered; on a native ledger those axioms are inert.The model is validated at registration: it must be an existing native ledger, and policies in it that use
f:queryare reported asmodel_warnings.Ergonomics
policy_enforcement.unevaluable_policieson the tracked response lists thef:queryids the source could not evaluate; the server logs a warning once per policy, and skipped triples maps log at debug.--default-allow/default_allowon a source keeps a model-less source readable under authentication, the counterpart of a native ledger'sf:defaultAllow. Without it, an authenticated server now returns nothing for a source with no model and no matching policy, which is the correct fail-closed outcome but a visible change on upgrade.fluree model access enable --connectedwarns when a registered source is governed by the model.modelanddefault-allow.Verification
it_iceberg_policychecks every static shape and pattern form against a native twin of the same fixture as the oracle, plus fail-closedf:query, column-derived classes, dataset mode, the model-ledger flow, registration validation, the tracked-response field, and thedefault-allowknob. Existing R2RML, Iceberg, SQL, policy, cross-ledger and reasoning suites pass.Not in this PR
opts.policyfor native ledgers, so a native ledger read through proxy mode is unfiltered. Pre-existing and out of scope here; this PR narrows it rather than widening it. Follow-up: graph-scoped query builder ignores opts.policy for native ledgers #1766