Uh oh!
There was an error while loading. Please reload this page.
Resolve group eNames to their members, by eName or profile id - #1123
Merged
Conversation
Contributor
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 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 |
Uh oh!
There was an error while loading. Please reload this page.
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Follow-up to #1122. A group eName in a policy now resolves to the eNames of its members, so naming a group in a grant or denial actually does something.
Closes the fail-open flagged in #1122: a denial naming a group previously matched nobody and silently failed to deny.
Both member reference forms
A group's participant list names members two different ways in practice, and the resolver accepts both because both are already in the data:
GroupManifest.membersGroup.participantIds(users(participants[].id),participantIdsin the platform mappings)A profile id resolves through that record's own
enamefield where it has one, falling back to the vault the record lives in. The record's own statement wins because the same profile syncs into several vaults, so the vault it happens to sit in does not reliably identify its subject. An id resolving to nothing is skipped rather than counted.Group records are found either in the group's own vault or by an
enamefield naming the group, across the Group/Chat (550e8400-…-440003) and GroupManifest (a8bfb7cf-…) ontologies. Participants are read frommembers,memberIds,participants,participantIds,adminsandownerand unioned — so admins and the owner count as members.Undeterminable membership is not "not a member"
The distinction matters because the two cases lead to opposite answers, and collapsing them would be wrong in one direction or the other:
So a group whose record cannot be read hands out nothing and stops removing nothing. A resolver that throws yields
unknownrather thanno.With no resolver configured at all, groups match nobody — that is the feature switched off rather than a lookup that failed, and it keeps every pre-existing decision unchanged.
Resolution is skipped when it cannot matter
A user or platform grant outranks any group grant, so when a direct grant matches, no group is resolved at all — there is a test asserting zero resolver calls in that case. Within one decision each group is resolved at most once, including across a grant and a denial naming it.
API shape
evaluate's fourth argument moves from a bareConditionEvaluatorto{ conditions?, groups? }, since there are now two collaborators.mostSpecificGrantbecomes async.Testing
enameand vault-fallback paths, dangling ids, scalarowner, every participant field unioned, deduplication, unknown groups, non-eName input, and non-group records.249/249 across 19 files. Typecheck clean; docs build passes with
onBrokenLinks: 'throw'.Docs
The group limitation is removed from the protocol doc, the post-platform guide, and the bundled
skills/w3dsreference, replaced with how resolution works and both reference forms. Adds "How a group resolves" and "When membership cannot be determined" to the protocol doc, "Naming a group instead of a person" to the implementation guide, and extends the glossary's Group entry.Still not in scope
Condition evaluation remains an unconnected seam, and enforcement is still eVault-side only. Group resolution reads records this eVault holds, so a group that has not synced here is undeterminable — which the docs spell out in both directions.
https://claude.ai/code/session_01UpwygDu2cizLp12tvvKqVZ