fix(ogar-codebook): restore COUNT_FUSE parity + classid-keyed authorize() probe#598
Merged
Merged
Conversation
…USE parity OGAR ogar-vocab #110 minted the 0x0B AuthStore family (auth_store/auth_zitadel/ auth_zanzibar/auth_ory_keto), taking class_ids::ALL from 39 to 43. The zero-dep lance-graph-contract::ogar_codebook mirror was not updated in the same arc, so the compile-time COUNT_FUSE in lance-graph-ogar fired error[E0080] in every consumer vendoring the OGAR git dep (medcare CI red on cargo build). - ogar_codebook: +4 auth CODEBOOK rows, ConceptDomain::Auth, 0x0B => Auth - lance-graph-ogar: domains_agree (O::Auth, C::Auth) arm (runtime parity) - board: ISS-OGAR-AUTH-MIRROR-DRIFT (resolved) + E-CODEBOOK-MINT-IS-A-CROSS-REPO-ARC Restores 43 == 43; mint provenance confirmed by OGIT Configuration |= auth_store. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EYvNjD8M8LMNYbRy3gq2FP
…e classes
Two latent issue-classes surfaced this session generalize across every
lance-graph/OGAR consumer:
A. codebook-mirror drift (the COUNT_FUSE break) — only the zero-dep
contract mirror can structurally drift; everything else re-exports.
B. PaaS deploy crash ( bind + writable data dir) — the medcare
Railway crash class, applicable to every web-app consumer.
Grounded per-client checklist with rg scan commands, verified status for
medcare (fixed) and op-canon (re-export, clean), and the canonical fix
patterns to copy.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EYvNjD8M8LMNYbRy3gq2FP
…can drift Workspace-wide id-table grep confirms exactly two authoritative codebook tables: OGAR/ogar-vocab (source) + lance-graph-contract::ogar_codebook (mirror, fused at 43). medcare/smb/woa/q2/ladybug/crewai/n8n/rs-graph-llm all re-export or pull via PortSpec — no hand-maintained copy. All Class-A rows ticked clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EYvNjD8M8LMNYbRy3gq2FP
OGAR keystone CLASSID-RBAC-KEYSTONE-SPEC.md step-4 gate: prove the
classid-keyed authorize() reproduces the shipped membrane gate
(Policy::evaluate) bit-for-bit before consumer collapse.
- authorize.rs: ClassRbac trait (§4), authorize() positive ∧ op-gate
kernel (§5), ClassGrants = PermissionSpec re-keyed by ClassId (§11).
Deny reasons mirror the shipped gate exactly → bit-for-bit parity.
- probe_ogar_rbac_authorize: 15-tuple corpus, all roles/ops/reasons +
depth boundary + unknown actor → all equal Policy::evaluate. GREEN.
- probe_is_falsifiable_under_wrong_keying: wrong classid flips Allow →
the gate is not vacuous.
Fence: certifies the positive ∧ op-gate half + classid re-keying against
the in-repo reference (positive role→permission only). Row-scope predicate
+ projecting Allow{scope,mask} (§5 stage 2) remain keystone work; the
scope-bearing references (Odoo ir.rule, OpenFGA) are the follow-on probes.
Promotes keystone §5 CONJECTURE->FINDING for the shipped reference;
unblocks the medcare #169 consumer-collapse on the positive-grant half.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EYvNjD8M8LMNYbRy3gq2FP
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
More reviews will be available in 45 minutes and 50 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses rolling per-developer review limits. Reviews become available again as older review attempts age out of the rolling limit window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds the ChangesAuth domain codebook mint and classid-keyed RBAC authorize kernel
Sequence Diagram(s)sequenceDiagram
participant Caller
participant authorize
participant ClassGrants
participant Policy
rect rgba(70, 130, 180, 0.5)
Note over Caller,ClassGrants: New classid-keyed path
Caller->>authorize: (actor, class_id: 0x0B01, op)
authorize->>ClassGrants: actor_roles(actor)
ClassGrants-->>authorize: [RoleId, ...]
authorize->>ClassGrants: grant_permits(role, 0x0B01, op)
ClassGrants-->>authorize: bool
authorize-->>Caller: AccessDecision
end
rect rgba(180, 100, 60, 0.5)
Note over Caller,Policy: Probe bit-for-bit reconciliation
Caller->>Policy: evaluate(actor, "auth_store", op)
Policy-->>Caller: AccessDecision
Caller->>Caller: assert eq!(authorize_result, policy_result)
Caller->>authorize: wrong class_id for same tuple
authorize-->>Caller: differing AccessDecision
Caller->>Caller: assert ne!(wrong_keying_result, policy_result)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
…-lance-graph-wmx76z # Conflicts: # .claude/board/ISSUES.md # crates/lance-graph-contract/src/ogar_codebook.rs
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/lance-graph-rbac/src/authorize.rs`:
- Around line 137-165: The methods with_actor and with_grant allow duplicate
keys to be appended to memberships and grants without replacing existing
entries, while the methods actor_roles and grant_for only return the first
match. This causes later updates to the same actor or (role, class) pair to be
silently ignored. Implement upsert semantics by modifying with_actor to check if
the actor already exists in memberships and update its roles instead of
appending, and similarly modify with_grant to check if the (role, class) pair
already exists in grants and update its permission instead of appending. This
ensures that re-assigning an actor or grant properly replaces the previous
assignment rather than shadowing it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e6272ba7-e220-411d-93c0-b5f478d4b4b1
📒 Files selected for processing (7)
.claude/board/CONSUMER_SCAN_TODO.md.claude/board/EPIPHANIES.md.claude/board/ISSUES.mdcrates/lance-graph-contract/src/ogar_codebook.rscrates/lance-graph-ogar/src/lib.rscrates/lance-graph-rbac/src/authorize.rscrates/lance-graph-rbac/src/lib.rs
Comment on lines
+137
to
+165
| /// Add a `(role, class) → grant` row (the re-keyed `PermissionSpec`). | ||
| #[must_use] | ||
| pub fn with_grant(mut self, role: RoleId, class: ClassId, grant: PermissionSpec) -> Self { | ||
| self.grants.push((role, class, grant)); | ||
| self | ||
| } | ||
|
|
||
| /// Assign an actor a set of roles (membership fold). | ||
| #[must_use] | ||
| pub fn with_actor(mut self, actor: &'static str, roles: Vec<RoleId>) -> Self { | ||
| self.memberships.push((actor, roles)); | ||
| self | ||
| } | ||
|
|
||
| fn grant_for(&self, role: RoleId, class: ClassId) -> Option<&PermissionSpec> { | ||
| self.grants | ||
| .iter() | ||
| .find(|(r, c, _)| *r == role && *c == class) | ||
| .map(|(_, _, g)| g) | ||
| } | ||
| } | ||
|
|
||
| impl ClassRbac for ClassGrants { | ||
| fn actor_roles(&self, actor: ActorId<'_>) -> &[RoleId] { | ||
| self.memberships | ||
| .iter() | ||
| .find(|(a, _)| *a == actor) | ||
| .map(|(_, roles)| roles.as_slice()) | ||
| .unwrap_or(&[]) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Prevent duplicate keys from silently shadowing authorization state.
with_actor / with_grant append duplicate keys, while actor_roles / grant_for read the first match. Re-inserting the same actor or (role, class) can silently ignore later updates and yield incorrect (including over-permissive) decisions.
Suggested fix (upsert semantics)
pub fn with_grant(mut self, role: RoleId, class: ClassId, grant: PermissionSpec) -> Self {
- self.grants.push((role, class, grant));
+ if let Some((_, _, existing)) = self
+ .grants
+ .iter_mut()
+ .find(|(r, c, _)| *r == role && *c == class)
+ {
+ *existing = grant;
+ } else {
+ self.grants.push((role, class, grant));
+ }
self
}
@@
pub fn with_actor(mut self, actor: &'static str, roles: Vec<RoleId>) -> Self {
- self.memberships.push((actor, roles));
+ if let Some((_, existing_roles)) = self.memberships.iter_mut().find(|(a, _)| *a == actor) {
+ *existing_roles = roles;
+ } else {
+ self.memberships.push((actor, roles));
+ }
self
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /// Add a `(role, class) → grant` row (the re-keyed `PermissionSpec`). | |
| #[must_use] | |
| pub fn with_grant(mut self, role: RoleId, class: ClassId, grant: PermissionSpec) -> Self { | |
| self.grants.push((role, class, grant)); | |
| self | |
| } | |
| /// Assign an actor a set of roles (membership fold). | |
| #[must_use] | |
| pub fn with_actor(mut self, actor: &'static str, roles: Vec<RoleId>) -> Self { | |
| self.memberships.push((actor, roles)); | |
| self | |
| } | |
| fn grant_for(&self, role: RoleId, class: ClassId) -> Option<&PermissionSpec> { | |
| self.grants | |
| .iter() | |
| .find(|(r, c, _)| *r == role && *c == class) | |
| .map(|(_, _, g)| g) | |
| } | |
| } | |
| impl ClassRbac for ClassGrants { | |
| fn actor_roles(&self, actor: ActorId<'_>) -> &[RoleId] { | |
| self.memberships | |
| .iter() | |
| .find(|(a, _)| *a == actor) | |
| .map(|(_, roles)| roles.as_slice()) | |
| .unwrap_or(&[]) | |
| /// Add a `(role, class) → grant` row (the re-keyed `PermissionSpec`). | |
| #[must_use] | |
| pub fn with_grant(mut self, role: RoleId, class: ClassId, grant: PermissionSpec) -> Self { | |
| if let Some((_, _, existing)) = self | |
| .grants | |
| .iter_mut() | |
| .find(|(r, c, _)| *r == role && *c == class) | |
| { | |
| *existing = grant; | |
| } else { | |
| self.grants.push((role, class, grant)); | |
| } | |
| self | |
| } | |
| /// Assign an actor a set of roles (membership fold). | |
| #[must_use] | |
| pub fn with_actor(mut self, actor: &'static str, roles: Vec<RoleId>) -> Self { | |
| if let Some((_, existing_roles)) = self.memberships.iter_mut().find(|(a, _)| *a == actor) { | |
| *existing_roles = roles; | |
| } else { | |
| self.memberships.push((actor, roles)); | |
| } | |
| self | |
| } | |
| fn grant_for(&self, role: RoleId, class: ClassId) -> Option<&PermissionSpec> { | |
| self.grants | |
| .iter() | |
| .find(|(r, c, _)| *r == role && *c == class) | |
| .map(|(_, _, g)| g) | |
| } | |
| } | |
| impl ClassRbac for ClassGrants { | |
| fn actor_roles(&self, actor: ActorId<'_>) -> &[RoleId] { | |
| self.memberships | |
| .iter() | |
| .find(|(a, _)| *a == actor) | |
| .map(|(_, roles)| roles.as_slice()) | |
| .unwrap_or(&[]) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/lance-graph-rbac/src/authorize.rs` around lines 137 - 165, The methods
with_actor and with_grant allow duplicate keys to be appended to memberships and
grants without replacing existing entries, while the methods actor_roles and
grant_for only return the first match. This causes later updates to the same
actor or (role, class) pair to be silently ignored. Implement upsert semantics
by modifying with_actor to check if the actor already exists in memberships and
update its roles instead of appending, and similarly modify with_grant to check
if the (role, class) pair already exists in grants and update its permission
instead of appending. This ensures that re-assigning an actor or grant properly
replaces the previous assignment rather than shadowing it.
The OGIT-imported auth class (arago auth_store = OGAR 0x0B01) is now the front-door of the authorization kernel (keystone §7 / I-K7: the inner authorize() never touches a token). - auth.rs: AuthProvider enum = the 0x0B family; classid resolved through the zero-dep contract mirror (no hardcoded 0x0B0N, no ogar-vocab dep). - ClaimGrammar per provider (Zitadel project-roles URN, Zanzibar tuple grammar, plain-OIDC base) — §7 'claim grammar as data'. - AuthProvider::resolve -> ResolvedIdentity (sub->actor, role-key->roles, org->tenant); ResolvedIdentity feeds authorize(). Token extraction stays at the consumer membrane (no JWT/serde dep in the rbac tier). - 21 tests green incl. resolved_identity_feeds_authorize (membrane->kernel). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EYvNjD8M8LMNYbRy3gq2FP
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Four commits, two arcs.
1. Unbreak every consumer (the build fix)
OGAR
ogar-vocabminted the0x0BAuthStore family (merged to OGARmain), takingclass_ids::ALLfrom 39 → 43. The zero-deplance-graph-contract::ogar_codebookmirror was not updated in the same arc, so the compile-timeCOUNT_FUSEinlance-graph-ogarfirederror[E0080]in every consumer vendoring the OGAR git dep — medcare CI went red oncargo build.472f840— mirror +4 auth rows (auth_store0x0B01 …auth_ory_keto0x0B04),ConceptDomain::Auth,0x0B => Auth, and the(O::Auth, C::Auth)arm inlance-graph-ogar::parity::domains_agree. 43 == 43 restored;cargo test -p lance-graph-contractgreen.2ab7589+31e421e—CONSUMER_SCAN_TODO.md: a grounded cross-repo scan checklist for the two latent issue classes (codebook-mirror drift + PaaS deploy crash), plus the Class-A sweep result: a workspace-wide grep confirms exactly two authoritative id-tables (OGAR source + this mirror); every other consumer re-exports or pulls via PortSpec. The drift surface is the one mirror.2. RBAC keystone step-4 gate (the OGAR
CLASSID-RBAC-KEYSTONE-SPEC.md)0414cde—lance-graph-rbac/src/authorize.rs:ClassRbactrait (§4),authorize()positive ∧ op-gate kernel (§5),ClassGrants=PermissionSpecre-keyed byClassId(§11).PROBE-OGAR-RBAC-AUTHORIZEreproduces the shipped membrane gate (Policy::evaluate) bit-for-bit (deny-reasons included) over a 15-tuple corpus; a falsifiability self-check proves the gate is not vacuous.Scope / honest fence
The RBAC probe certifies the positive ∧ op-gate half + classid re-keying against the in-repo reference (positive role→permission only) → keystone §5 promoted CONJECTURE→FINDING for that reference. The §5 stage-2 row-scope predicate + projecting
Allow{scope,mask}stay CONJECTURE — the scope-bearing references (Odooir.rule, OpenFGA) are the follow-on probes.Tests
cargo test -p lance-graph-contract— green (ogar_codebook drift guards).cargo test -p lance-graph-rbac— 17 passed incl.probe_ogar_rbac_authorize+probe_is_falsifiable_under_wrong_keying.cargo clippy -p lance-graph-rbac— clean.Board hygiene
EPIPHANIES.mdprepended:E-CODEBOOK-MINT-IS-A-CROSS-REPO-ARC,E-RBAC-AUTHORIZE-PROBE-GREEN.ISSUES.md:ISS-OGAR-AUTH-MIRROR-DRIFT(resolved).Cross-repo note
Pairs with AdaWorldAPI/OGAR PR (keystone §10 step-4 status + the nine-domain promotion decision). This branch is what medcare CI vendors via its
vendor/lance-graphsymlink.🤖 Generated with Claude Code
Generated by Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation