Repository navigation
ogar-doc-ir: resolve documents under a reader's access decision - #359
Conversation
Documents now resolve under a reader's access decision. The new entry points take allowed(class) -> WideFieldMask (lance-graph-rbac's visible_fields(), EMPTY on a refused decision) and walk through lance-graph's walk_rails_authorized: a hidden field is not emitted at any depth and a hidden rail is not followed. A root the reader may not read resolves like a missing object (fallback, else Unresolvable) instead of an empty Resolved, and a grid view needs a readable root. resolve_slot and resolve_doc are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G22yT6htkcdyXsihxxXdrg
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G22yT6htkcdyXsihxxXdrg
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
OGAR/crates/ogar-doc-ir/src/resolve.rs
Lines 369 to 374 in e6a0e1d
For an authorized grid request, any nonempty class mask passes the root check, after which this branch returns every axis label, measure, and cell from grid_of without intersecting them with allowed. Thus a reader permitted to see only one harmless field can receive an entire grid containing values outside that field grant—the added test even demonstrates that bit 0 alone unlocks the complete grid. Until grid dimensions and measures have mask positions, authorized resolution should fail closed or require an explicit full-grid permission rather than treating any readable field as permission for all cells.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Two gaps codex found in the authorized entry points:
- The field mask is per class, so it cannot tell two objects of one class
apart. The entry points now take Access { fields, admits }: admits(key)
is the decision's row scope. A root outside it resolves like a missing
object, and through a scoped view of the graph an object outside it has
no fields and no rail leads to it.
- A grid has no field positions, so any readable field opened every cell.
Under an access decision a grid view is now refused (fallback, else
Unresolvable); grids resolve only through resolve_slot.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G22yT6htkcdyXsihxxXdrg
A disable run showed the rail_target filter is not load-bearing: an object outside the scope already has no present fields, so the walk emits nothing for it and follows none of its rails. The doc comments now say that. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G22yT6htkcdyXsihxxXdrg
|
On the Codex review's grid finding (any readable field returned the whole grid): agreed. Fixed in 4804b72. Grid cells have no field positions, so no field grant can select among them. Under an access decision, a grid view is now refused: it returns the fallback if there is one, otherwise
Generated by Claude Code |
What
Documents can now be resolved for a specific reader. Two new entry points,
resolve_slot_authorizedandresolve_doc_authorized, take anAccess { fields, admits }:fields(class) -> WideFieldMaskis the set of field positions the reader may see, per class. Callers should pass lance-graph-rbac'svisible_fields(), which isEMPTYon a refused decision.admits(key) -> boolis the decision's row scope. The field mask is per class, so this is what tells two objects of the same class apart.Both entry points walk through lance-graph's
walk_rails_authorized, merged in lance-graph#1558, so:Unresolvablewhen there is none. It does not returnResolvedwith no fields, which would say the object exists and is empty.resolve_slot.resolve_slotandresolve_docare unchanged. Internally both entry points delegate to oneresolve_slot_with, with no decision.Tests
Seven new tests in
resolve::tests:an_authorized_resolve_hides_a_nested_field_the_reader_may_not_seean_authorized_resolve_does_not_follow_a_hidden_railan_unreadable_root_resolves_like_a_missing_objecta_grid_view_is_refused_under_an_access_decisionan_object_outside_the_row_scope_resolves_like_a_missing_onea_rail_to_an_object_outside_the_row_scope_is_not_followedresolve_doc_authorized_applies_the_mask_to_every_slotDisable runs show each check is needed:
A rail-target filter in the scoped graph turned out to be redundant: an out-of-scope object already has no fields, so the walk emits nothing and goes no further through it. That filter was removed.
Local checks:
cargo testpasses forogar-doc-ir,ogar-render-typst,ogar-from-docv1andogar-a2ui-frame.cargo check --workspace --all-targetsis clean.clippy -D warningsandfmt --checkare clean.🤖 Generated with Claude Code
https://claude.ai/code/session_01G22yT6htkcdyXsihxxXdrg
Generated by Claude Code