feat: Add authorization models for vertical industries#70
Merged
Conversation
Contributor
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughAdds a large set of new OpenFGA sample stores: each store includes a README, an FGA model (model.fga), and a test suite (store.fga.yaml) covering domain-specific authorization models across many industries (accounting, chat, crm, healthcare, ecommerce, etc.). Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds new OpenFGA sample stores for common vertical industry use cases, each including an authorization model plus runnable CLI tests and a short README to explain intent and usage.
Changes:
- Introduces 8 new sample-store directories (Accounting, ATS, Chat, CRM, File Storage, HR, Issue Tracking, Knowledge Base).
- Adds
.fgaauthorization models demonstrating multi-tenancy, role-based access, hierarchy inheritance, and ownership patterns. - Adds
.fga.yamltest suites and per-store README docs describing scenarios and how to runfga model test.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| stores/accounting/model.fga | Accounting authorization model (roles + financial artifacts). |
| stores/accounting/model.fga.yaml | CLI model tests + sample tuples for accounting scenarios. |
| stores/accounting/README.md | Explains accounting use case and how to run tests. |
| stores/applicant-tracking-system/model.fga | ATS authorization model (jobs, candidates, applications, interviews, etc.). |
| stores/applicant-tracking-system/model.fga.yaml | CLI model tests + sample tuples for ATS scenarios. |
| stores/applicant-tracking-system/README.md | Explains ATS use case and how to run tests. |
| stores/chat/model.fga | Chat authorization model (orgs, groups, conversations, messages). |
| stores/chat/model.fga.yaml | CLI model tests + sample tuples for chat scenarios. |
| stores/chat/README.md | Explains chat use case and how to run tests. |
| stores/crm/model.fga | CRM authorization model (accounts, contacts, leads, opportunities, etc.). |
| stores/crm/model.fga.yaml | CLI model tests + sample tuples for CRM scenarios. |
| stores/crm/README.md | Explains CRM use case and how to run tests. |
| stores/file-storage/model.fga | File storage authorization model (drives/folders/files + inheritance). |
| stores/file-storage/model.fga.yaml | CLI model tests + sample tuples for file storage scenarios. |
| stores/file-storage/README.md | Explains file storage use case and how to run tests. |
| stores/human-resources/model.fga | HRIS authorization model (employees, teams, payroll, benefits, etc.). |
| stores/human-resources/model.fga.yaml | CLI model tests + sample tuples for HR scenarios. |
| stores/human-resources/README.md | Explains HR use case and how to run tests. |
| stores/issue-tracking/model.fga | Issue tracking model (collections, tickets, comments, attachments, contacts). |
| stores/issue-tracking/model.fga.yaml | CLI model tests + sample tuples for issue tracking scenarios. |
| stores/issue-tracking/README.md | Explains issue tracking use case and how to run tests. |
| stores/knowledge-base/model.fga | Knowledge base model (containers, articles, attachments + inheritance/public). |
| stores/knowledge-base/model.fga.yaml | CLI model tests + sample tuples for knowledge base scenarios. |
| stores/knowledge-base/README.md | Explains knowledge base use case and how to run tests. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (17)
stores/crm/model.fga.yaml (1)
70-186: Consider adding test cases forcan_deleteon contacts and leads.The model defines
can_delete: admin from organizationfor bothcontactandleadtypes, but these permissions aren't tested. Adding assertions would ensure complete coverage of admin-only delete permissions.💡 Example test cases to add
- name: Admin can delete contacts and leads check: - user: user:alice object: contact:john-doe assertions: can_delete: true - user: user:alice object: lead:new-prospect assertions: can_delete: true - user: user:charlie object: contact:john-doe assertions: can_delete: false - user: user:charlie object: lead:new-prospect assertions: can_delete: false🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stores/crm/model.fga.yaml` around lines 70 - 186, Add tests that assert can_delete behavior for contact and lead to cover the model rule "can_delete: admin from organization": add a new test block (e.g., name: Admin can delete contacts and leads) that checks user: user:alice against object: contact:john-doe and object: lead:new-prospect with assertions can_delete: true, and also assert non-admin (e.g., user: user:charlie) for those same objects has can_delete: false; reference the existing test file's patterns (tests array, contact, lead, can_delete) to place and format the new checks consistently.stores/knowledge-base/model.fga.yaml (2)
4-72: Theparent_articlerelation is not tested.The model defines a
parent_articlerelation on thearticletype, but no tuples establish this relationship and no tests verify its behavior. Based on learnings, unused relations should be either tested or removed from the model.Consider either:
- Adding tuples and tests to exercise
parent_article(e.g., for sub-pages or linked articles), or- Removing
parent_articlefrommodel.fgaif it's not needed for this sample.Based on learnings: "Remove from the model all types and relations that are not referenced in the model or the tests after generating model and tests"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stores/knowledge-base/model.fga.yaml` around lines 4 - 72, The model defines an unused relation parent_article on the article type but no tuples or tests exercise it; either add tuples and tests that create and assert parent_article behavior (e.g., create article:parent-child relations and update unit tests to check inheritance/visibility via parent_article) or remove the parent_article relation from the model to avoid dead schema. Locate the article type and the parent_article relation in the model (symbol: parent_article on type article) and choose one of the two actions: (A) add example tuples linking article:parent -> article:child and corresponding test cases that assert expected access/role resolution through parent_article, or (B) remove the parent_article relation declaration and any references in tests to keep the model and tests consistent.
74-167: Consider addinglist_objectsandlist_userstests for more comprehensive coverage.The test suite currently only includes
checkassertions. Per coding guidelines,.fga.yamltest files should also includelist_objectsandlist_userstests to validate the model from different query perspectives.Example additions:
- name: List containers Bob can edit list_objects: - user: user:bob type: container assertions: can_edit: - container:engineering-space - container:api-docs - name: List users who can view getting-started article list_users: - object: article:getting-started user_filter: - type: user assertions: can_view: - user:alice - user:bob - user:charlieAs per coding guidelines: "Define test cases for OpenFGA models in the tests section of .fga.yaml files, including check, list_objects, and list_users tests"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stores/knowledge-base/model.fga.yaml` around lines 74 - 167, Add list_objects and list_users test cases to the existing tests section to cover query perspectives: add a list_objects test named like "List containers Bob can edit" using user: user:bob and type: container with assertions for can_edit returning container:engineering-space and container:api-docs (matching the "Nested container inherits from parent" and group-based access checks), and add a list_users test named like "List users who can view getting-started article" using object: article:getting-started with user_filter type: user and assertions for can_view listing user:alice, user:bob, and user:charlie (matching the article/container visibility checks); place these entries alongside the existing check tests in the tests block so they run with the other scenarios.stores/knowledge-base/model.fga (1)
33-36:parent_articlerelation is defined but unused.The
parent_articlerelation is declared but never referenced in any computed permission. Theeditorandviewerrelations only inherit fromparent_container, not fromparent_article.If article-to-article hierarchy is intended (e.g., sub-pages inheriting permissions from a parent article), consider adding inheritance:
- define editor: [user, group#member] or author or editor from parent_container - define viewer: [user, group#member, user:*] or editor or viewer from parent_container + define editor: [user, group#member] or author or editor from parent_container or editor from parent_article + define viewer: [user, group#member, user:*] or editor or viewer from parent_container or viewer from parent_articleOtherwise, remove the unused relation to keep the model clean.
Based on learnings: "Remove from the model all types and relations that are not referenced in the model or the tests after generating model and tests"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stores/knowledge-base/model.fga` around lines 33 - 36, The model declares define parent_article: [article] but it is never used; either remove that unused relation or make it part of the inheritance chain for article permissions—update the definitions of editor and viewer (the editor and viewer relations) to include inheritance from parent_article (e.g., add "or editor or viewer from parent_article" to their expressions) if you intend article-to-article permission inheritance, otherwise delete the define parent_article line to keep the model clean.stores/accounting/model.fga (1)
27-35: Consider removing the unusedcontactrelation.The
contactrelation oninvoice(line 31) is defined but not referenced in any permission computation or tested. If it's not planned for future use, removing it would keep the model cleaner.Proposed fix
type invoice relations define organization: [organization] define creator: [user] - define contact: [contact] define can_view: creator or accountant from organization or admin from organization or auditor from organization define can_edit: creator or accountant from organization or admin from organization define can_approve: admin from organization define can_void: admin from organizationBased on learnings: "Remove from the model all types and relations that are not referenced in the model or the tests after generating model and tests."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stores/accounting/model.fga` around lines 27 - 35, Remove the unused relation by deleting the "define contact: [contact]" line from the invoice type in the FGA model (symbol: type invoice, relation: contact); ensure no other rules, permissions (can_view, can_edit, can_approve, can_void) or tests reference "contact" before removal, and run the model/test generation/check steps to confirm no references remain.stores/accounting/model.fga.yaml (1)
66-198: Good test coverage; consider addinglist_objectsorlist_userstests.The
checktests comprehensively verify role-based permissions across all user types. For enhanced coverage, you could addlist_objectstests (e.g., list all invoices a user can view) orlist_userstests (e.g., list users who can approve a purchase order).Example list_objects test
- name: List objects accountant can view list_objects: - user: user:bob type: invoice assertions: can_view: - invoice:inv-001As per coding guidelines: "Define test cases for OpenFGA models in the tests section of .fga.yaml files, including check, list_objects, and list_users tests."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stores/accounting/model.fga.yaml` around lines 66 - 198, Add list_objects and/or list_users style tests to the tests block to exercise listing semantics in addition to the existing check assertions; for example add a "List objects accountant can view" entry that uses list_objects for user:user:bob with type: invoice and an assertions.can_view list including invoice:inv-001, and similarly consider a "List users who can approve purchase order" entry using list_users for object: purchase_order:po-001 asserting user: user:bob appears; update the same tests section (near the existing "Purchase order flow" and "Journal entry permissions" entries) so the OpenFGA test runner validates listing behavior as per the coding guideline.stores/chat/model.fga.yaml (1)
72-188: Comprehensive check test coverage.The test suite thoroughly covers the permission model across all scenarios: ownership, group membership, DM participation, message authorship, admin override, and non-member isolation.
As per coding guidelines, consider adding
list_objectsand/orlist_userstests to demonstrate listing capabilities (e.g., listing all conversations a user can view, or listing all users who can view a specific message).💡 Example list_objects test
- name: List conversations user can view list_objects: - user: user:bob type: conversation assertions: can_view: - conversation:engineering-general - conversation:bob-charlie-dm🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stores/chat/model.fga.yaml` around lines 72 - 188, Add tests demonstrating listing capabilities by extending the tests array with entries using the list_objects and/or list_users keys; for example add a test named "List conversations user can view" using list_objects for user:bob and type:conversation asserting can_view includes conversation:engineering-general and conversation:bob-charlie-dm, and add a "List users who can view message" test using list_users for object:message:msg-001 asserting expected users; ensure the new tests follow the same YAML structure and naming convention as existing cases (e.g., "List conversations user can view", "List users who can view message") so they integrate with the current test harness.stores/issue-tracking/model.fga (1)
40-42: Design consideration: Agentcan_closepermission.The README states agents can "manage tickets," but
can_closecurrently excludes agents (only assignee, team lead, or admin can close). If agents should be able to close any ticket in their organization, consider addingagent from organizationtocan_close. If this is intentional (agents must be assigned to close), the current design is correct—just worth documenting.🔧 Optional: Allow agents to close tickets
- define can_close: assignee or lead from assigned_team or admin from organization + define can_close: assignee or lead from assigned_team or agent from organization or admin from organization🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stores/issue-tracking/model.fga` around lines 40 - 42, The can_close permission currently allows only assignee, lead from assigned_team, or admin from organization to close tickets; if agents should be able to close any ticket in their organization, update the can_close definition to include "agent from organization" (i.e., add the symbol agent from organization to the existing define can_close rule) so agents gain closure rights across the org; if this was intentional, add documentation in the README clarifying that can_close excludes general agents and only allows assignees, team leads, or org admins.stores/applicant-tracking-system/model.fga (3)
24-32: Unuseddepartmentrelation on job type.The
department: [department]relation is defined but never referenced in permissions or tested in the YAML file. Based on learnings: "Remove from the model all types and relations that are not referenced in the model or the tests after generating model and tests."Consider either:
- Removing this relation if not needed
- Adding a tuple and test case that exercises department-based job access
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stores/applicant-tracking-system/model.fga` around lines 24 - 32, The model defines an unused relation "department" on the "job" type; either remove the line "define department: [department]" from the job type in model.fga, or add integration by creating a tuple that links a job to a department and a corresponding test in the YAML that asserts department-based access (e.g., grant a user a role/permission on department and verify it affects job access), and update any relevant permission rules (can_view / can_edit / can_close) or tests to reference "department" so the relation is exercised.
52-76: Interview, scorecard, and offer types have unused parent relations.The
applicationrelation onscheduled_interviewandoffer, plusinterviewonscorecard, are defined but not referenced in permissions. While these may be useful for data modeling, they aren't exercised in authorization logic.If these are intentionally kept for future permission inheritance or data traversal, consider adding a comment explaining the design intent. Otherwise, per learnings, unused relations should be removed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stores/applicant-tracking-system/model.fga` around lines 52 - 76, The FGA types define unused parent relations—scheduled_interview.application, offer.application, and scorecard.interview—that are not referenced by any permission rules; either remove these unused relation definitions to avoid dead schema, or add a concise comment above each (scheduled_interview, scorecard, offer) explaining they are intentionally preserved for future permission inheritance or data traversal so reviewers understand the design intent; update the model.fga entries for the symbols scheduled_interview.application, scorecard.interview, and offer.application accordingly.
42-50: Unusedcandidaterelation on application type.The
candidate: [candidate]relation is defined but not referenced in any permission definitions or tested. Consider removing it or adding permission logic that derives access from the linked candidate.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stores/applicant-tracking-system/model.fga` around lines 42 - 50, The application type declares an unused relation candidate: [candidate]; either remove that relation from the application type or incorporate it into permissions so it is used (for example add candidate-based access like "or owner from candidate" or "or candidate from candidate" into define can_view / can_edit / can_change_stage). Update the type application relations block (remove the line "define candidate: [candidate]" if deleting) or update the permission lines (e.g., modify define can_view to include "or owner from candidate") and ensure the referenced relation name candidate matches the candidate model's relation (e.g., owner) so the FGA rules compile and reflect intended candidate-derived access.stores/applicant-tracking-system/model.fga.yaml (1)
86-197: Test coverage is solid with correct permission assertions.The test cases effectively verify the key authorization scenarios: admin full access, role-scoped permissions, candidate privacy (hiring manager cannot view directly), scorecard confidentiality, and offer approval restrictions.
Consider adding
list_objectsorlist_userstests to complement thecheckassertions, which can help validate relationship traversal from different angles. As per coding guidelines: "Define test cases for OpenFGA models in the tests section of .fga.yaml files, including check, list_objects, and list_users tests."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stores/applicant-tracking-system/model.fga.yaml` around lines 86 - 197, Add complementary list_objects and list_users checks to the tests block to validate relationship traversal for key scenarios: for "Admin has full access to jobs" add a list_objects test asserting list_objects(user:alice, object_type:job) returns job:swe-senior; for "Recruiter can view and edit candidates" add list_objects(user:bob, object_type:candidate) returns candidate:diana and list_users(object:candidate:diana) includes user:bob; for "Hiring manager cannot directly view candidate" add list_objects(user:charlie, object_type:candidate) asserting candidate:diana is not returned and list_users(object:candidate:diana) does not include user:charlie; similarly add list_objects/list_users for application and offer scenarios (use objects application:diana-swe and offer:diana-offer) to ensure traversal-based permissions are covered.stores/human-resources/model.fga (1)
48-83: Several relations are declared but not used in permission derivations.
employee.company,employee.team,employment.employee, andtime_off.employeeare currently inert in access decisions; either wire them intocan_*rules or remove them to reduce model drift.Based on learnings: Applies to **/*.openfga : Remove from the model all types and relations that are not referenced in the model or the tests after generating model and tests.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stores/human-resources/model.fga` around lines 48 - 83, The model declares relations employee.company, employee.team, employment.employee, and time_off.employee but never uses them in any can_* derivations; either wire these relations into the relevant permission rules or delete them to avoid dead relations. Locate the relations on types employee, employment, and time_off in model.fga and either (A) add them to the appropriate can_view/can_edit/can_approve rules (for example include employment.employee and time_off.employee in can_view derivations so the referenced user/employee relation grants access, or use employee.company/employee.team to grant org/team-based manager or hr_admin access), or (B) remove the unused define lines and update corresponding tests to reflect the removal. Ensure tests that assert access via these relations are updated to use the new derivation or removed if the relation is deleted.stores/human-resources/model.fga.yaml (1)
147-155: Missing regression assertion for admin sensitive-data access.Please add an assertion for
user:aliceonemployee:diana-recordwithcan_view_sensitive: trueto lock expected admin behavior and catch regressions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stores/human-resources/model.fga.yaml` around lines 147 - 155, Add a regression assertion under the "Admin can terminate employees" check for the admin sensitive-data access: include an assertions entry for user: user:alice on object: employee:diana-record that sets can_view_sensitive: true so the policy explicitly locks expected admin behavior and will catch regressions; locate the check block with name "Admin can terminate employees" and extend its assertions to include the can_view_sensitive: true assertion alongside can_view, can_edit, and can_terminate.stores/file-storage/model.fga (1)
42-50: Remove or wire unusedfilerelations to permissions.
file.driveandfile.organizationare currently unused in permission derivations/tests, which adds dead surface area and tuple maintenance overhead.Suggested cleanup
type file relations define folder: [folder] - define drive: [drive] define owner: [user] or owner from folder define writer: [user, group#member] or owner or writer from folder define reader: [user, group#member] or writer or reader from folder define can_view: reader define can_edit: writer define can_download: reader define can_delete: owner - define organization: [organization]Based on learnings, "Applies to **/*.openfga : Remove from the model all types and relations that are not referenced in the model or the tests after generating model and tests".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stores/file-storage/model.fga` around lines 42 - 50, The model exposes unused relations file.drive and file.organization which are not referenced by any permission definitions (can_view, can_edit, can_download, can_delete) or tests; remove these dead relations from the file type (delete the lines defining file.drive and file.organization) or wire them into permission derivations if they are intended to matter (e.g., reference file.drive in owner/writer/reader derivations or include organization in a higher-level allow rule). Update the file type so only relations used by define can_view/can_edit/can_download/can_delete remain, and adjust any test fixtures to remove tuples for the removed relations.stores/file-storage/model.fga.yaml (1)
129-134: This test does not validate the stated “drive reader” path.At Line 131,
user:bobonfolder:projectspasses via folder ownership (Line 43-Line 45 tuples), so it does not isolate drive-reader inheritance or nested-folder behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stores/file-storage/model.fga.yaml` around lines 129 - 134, The test named "Drive reader can read nested folders" currently uses user:bob and folder:projects, but bob obtains access via folder ownership tuples so it doesn't exercise the drive-reader inheritance path; update this test (the "Drive reader can read nested folders" case that asserts can_view on folder:projects) to use a principal that only has the drive-reader role (e.g., a user like user:drive_reader or user:carol) and point at a nested folder object that demonstrates inheritance (for example a child folder such as folder:projects/subfolder or another nested folder) while ensuring no ownership tuples exist that would grant direct access to that principal. Ensure the assertion remains can_view so the test validates drive-reader inheritance rather than folder ownership.stores/file-storage/README.md (1)
22-24: Consider documenting both invocation contexts for the test command.You can make this more copy-paste friendly by showing both:
- from
stores/file-storage:fga model test --tests model.fga.yaml- from repo root:
fga model test --tests stores/file-storage/model.fga.yaml🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stores/file-storage/README.md` around lines 22 - 24, Update the README section that shows the test command to include two explicit, copy-paste friendly invocations: the current-directory invocation using the exact command shown ('fga model test --tests model.fga.yaml') and a repo-root invocation that references the same file via its relative path (the model.fga.yaml inside the file-storage directory), so readers can run the test either from inside the file-storage folder or from the repository root.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@stores/crm/model.fga.yaml`:
- Around line 158-165: The test case name "Note owner can edit but only admin
can delete" is misleading because the model's delete rule allows owner or admin
and the assertion expects can_delete: true for user:charlie; update the test
name to reflect that owners may delete (e.g., "Note owner can edit and delete;
admins can delete") and ensure the testcase entry (the `name` field) referencing
`user: user:charlie`, `object: note:meeting-notes`, and the `can_delete`
assertion is renamed accordingly so it accurately describes the expected
behavior.
In `@stores/file-storage/model.fga`:
- Around line 36-37: The folder's organization relation must be derived from its
container to avoid divergent tenancy; replace the writable relation `define
organization: [organization]` with a computed relation that sources the org from
the folder's container (e.g., derive `organization` from `folder.drive`/its
container relation) so `can_delete` uses `owner or admin from organization`
reliably; remove direct writes to `folder.organization` and instead map
`organization` to the container/drive's organization relation (referencing
`can_delete`, `organization`, `folder.organization`, and `folder.drive` to
locate the relevant definitions).
In `@stores/file-storage/model.fga.yaml`:
- Around line 67-134: Add two new test cases in the tests block: one
"list_objects" test and one "list_users" test to complement the existing "check"
tests (use the same subjects/objects from existing checks such as user:alice,
user:bob, user:charlie and objects drive:shared-drive, drive:bobs-drive,
folder:projects, folder:backend, file:api-spec). For the list_objects case,
query which objects a given user can list (e.g., list objects in
drive:shared-drive or list files in folder:projects) and assert expected
membership for can_view/can_edit where appropriate; for the list_users case,
query which users have a given permission on a specific object (e.g., who
can_view or can_delete drive:bobs-drive or file:api-spec) and assert expected
user entries (alice, bob, charlie). Ensure the new entries follow the same YAML
structure as other tests but use the "list_objects" and "list_users" test types
so the model is validated across all query types.
In `@stores/human-resources/model.fga`:
- Line 69: The rule benefit.can_view is currently grantable to any principal
because it includes the direct principal set [user]; remove [user] so ordinary
users cannot be directly granted this permission and instead rely on role-based
relations — update define can_view to only include hr_manager from organization
or admin from organization (or an explicit owner/member relation if self-access
is needed) so benefit.can_view is not directly assignable to the public
principal.
- Line 54: can_view_sensitive currently allows only assignee or hr_admin, which
blocks organization admins from reading sensitive employee fields; update the
definition of can_view_sensitive to include the organization admin relation
(e.g., add "or admin from organization") so that employee.can_view_sensitive
grants access to assignees, HR admins, and org admins; edit the
can_view_sensitive rule in model.fga (referencing can_view_sensitive, assignee,
hr_admin, and admin from organization) to include that additional OR clause.
In `@stores/human-resources/model.fga.yaml`:
- Around line 99-260: Add list_objects and list_users test blocks to the
existing tests section to satisfy the .fga.yaml test-shape requirement: for key
scenarios already covered by check tests (e.g. organization:acme,
employee:diana-record, team:engineering, payroll_run:march-2026,
time_off:diana-vacation) add corresponding list_objects cases that assert which
object IDs each user (user:alice, user:bob, user:charlie, user:diana, user:eve)
should see and list_users cases that assert which users are returned for given
objects/relations (e.g. list users with admin/hr_manager/manager/member/assignee
roles for organization:acme, team:engineering, employee:diana-record, and
payroll_run:march-2026); ensure each new test uses the same object/user
identifiers used in the existing check tests so expectations align with those
assertions.
---
Nitpick comments:
In `@stores/accounting/model.fga`:
- Around line 27-35: Remove the unused relation by deleting the "define contact:
[contact]" line from the invoice type in the FGA model (symbol: type invoice,
relation: contact); ensure no other rules, permissions (can_view, can_edit,
can_approve, can_void) or tests reference "contact" before removal, and run the
model/test generation/check steps to confirm no references remain.
In `@stores/accounting/model.fga.yaml`:
- Around line 66-198: Add list_objects and/or list_users style tests to the
tests block to exercise listing semantics in addition to the existing check
assertions; for example add a "List objects accountant can view" entry that uses
list_objects for user:user:bob with type: invoice and an assertions.can_view
list including invoice:inv-001, and similarly consider a "List users who can
approve purchase order" entry using list_users for object: purchase_order:po-001
asserting user: user:bob appears; update the same tests section (near the
existing "Purchase order flow" and "Journal entry permissions" entries) so the
OpenFGA test runner validates listing behavior as per the coding guideline.
In `@stores/applicant-tracking-system/model.fga`:
- Around line 24-32: The model defines an unused relation "department" on the
"job" type; either remove the line "define department: [department]" from the
job type in model.fga, or add integration by creating a tuple that links a job
to a department and a corresponding test in the YAML that asserts
department-based access (e.g., grant a user a role/permission on department and
verify it affects job access), and update any relevant permission rules
(can_view / can_edit / can_close) or tests to reference "department" so the
relation is exercised.
- Around line 52-76: The FGA types define unused parent
relations—scheduled_interview.application, offer.application, and
scorecard.interview—that are not referenced by any permission rules; either
remove these unused relation definitions to avoid dead schema, or add a concise
comment above each (scheduled_interview, scorecard, offer) explaining they are
intentionally preserved for future permission inheritance or data traversal so
reviewers understand the design intent; update the model.fga entries for the
symbols scheduled_interview.application, scorecard.interview, and
offer.application accordingly.
- Around line 42-50: The application type declares an unused relation candidate:
[candidate]; either remove that relation from the application type or
incorporate it into permissions so it is used (for example add candidate-based
access like "or owner from candidate" or "or candidate from candidate" into
define can_view / can_edit / can_change_stage). Update the type application
relations block (remove the line "define candidate: [candidate]" if deleting) or
update the permission lines (e.g., modify define can_view to include "or owner
from candidate") and ensure the referenced relation name candidate matches the
candidate model's relation (e.g., owner) so the FGA rules compile and reflect
intended candidate-derived access.
In `@stores/applicant-tracking-system/model.fga.yaml`:
- Around line 86-197: Add complementary list_objects and list_users checks to
the tests block to validate relationship traversal for key scenarios: for "Admin
has full access to jobs" add a list_objects test asserting
list_objects(user:alice, object_type:job) returns job:swe-senior; for "Recruiter
can view and edit candidates" add list_objects(user:bob, object_type:candidate)
returns candidate:diana and list_users(object:candidate:diana) includes
user:bob; for "Hiring manager cannot directly view candidate" add
list_objects(user:charlie, object_type:candidate) asserting candidate:diana is
not returned and list_users(object:candidate:diana) does not include
user:charlie; similarly add list_objects/list_users for application and offer
scenarios (use objects application:diana-swe and offer:diana-offer) to ensure
traversal-based permissions are covered.
In `@stores/chat/model.fga.yaml`:
- Around line 72-188: Add tests demonstrating listing capabilities by extending
the tests array with entries using the list_objects and/or list_users keys; for
example add a test named "List conversations user can view" using list_objects
for user:bob and type:conversation asserting can_view includes
conversation:engineering-general and conversation:bob-charlie-dm, and add a
"List users who can view message" test using list_users for
object:message:msg-001 asserting expected users; ensure the new tests follow the
same YAML structure and naming convention as existing cases (e.g., "List
conversations user can view", "List users who can view message") so they
integrate with the current test harness.
In `@stores/crm/model.fga.yaml`:
- Around line 70-186: Add tests that assert can_delete behavior for contact and
lead to cover the model rule "can_delete: admin from organization": add a new
test block (e.g., name: Admin can delete contacts and leads) that checks user:
user:alice against object: contact:john-doe and object: lead:new-prospect with
assertions can_delete: true, and also assert non-admin (e.g., user:
user:charlie) for those same objects has can_delete: false; reference the
existing test file's patterns (tests array, contact, lead, can_delete) to place
and format the new checks consistently.
In `@stores/file-storage/model.fga`:
- Around line 42-50: The model exposes unused relations file.drive and
file.organization which are not referenced by any permission definitions
(can_view, can_edit, can_download, can_delete) or tests; remove these dead
relations from the file type (delete the lines defining file.drive and
file.organization) or wire them into permission derivations if they are intended
to matter (e.g., reference file.drive in owner/writer/reader derivations or
include organization in a higher-level allow rule). Update the file type so only
relations used by define can_view/can_edit/can_download/can_delete remain, and
adjust any test fixtures to remove tuples for the removed relations.
In `@stores/file-storage/model.fga.yaml`:
- Around line 129-134: The test named "Drive reader can read nested folders"
currently uses user:bob and folder:projects, but bob obtains access via folder
ownership tuples so it doesn't exercise the drive-reader inheritance path;
update this test (the "Drive reader can read nested folders" case that asserts
can_view on folder:projects) to use a principal that only has the drive-reader
role (e.g., a user like user:drive_reader or user:carol) and point at a nested
folder object that demonstrates inheritance (for example a child folder such as
folder:projects/subfolder or another nested folder) while ensuring no ownership
tuples exist that would grant direct access to that principal. Ensure the
assertion remains can_view so the test validates drive-reader inheritance rather
than folder ownership.
In `@stores/file-storage/README.md`:
- Around line 22-24: Update the README section that shows the test command to
include two explicit, copy-paste friendly invocations: the current-directory
invocation using the exact command shown ('fga model test --tests
model.fga.yaml') and a repo-root invocation that references the same file via
its relative path (the model.fga.yaml inside the file-storage directory), so
readers can run the test either from inside the file-storage folder or from the
repository root.
In `@stores/human-resources/model.fga`:
- Around line 48-83: The model declares relations employee.company,
employee.team, employment.employee, and time_off.employee but never uses them in
any can_* derivations; either wire these relations into the relevant permission
rules or delete them to avoid dead relations. Locate the relations on types
employee, employment, and time_off in model.fga and either (A) add them to the
appropriate can_view/can_edit/can_approve rules (for example include
employment.employee and time_off.employee in can_view derivations so the
referenced user/employee relation grants access, or use
employee.company/employee.team to grant org/team-based manager or hr_admin
access), or (B) remove the unused define lines and update corresponding tests to
reflect the removal. Ensure tests that assert access via these relations are
updated to use the new derivation or removed if the relation is deleted.
In `@stores/human-resources/model.fga.yaml`:
- Around line 147-155: Add a regression assertion under the "Admin can terminate
employees" check for the admin sensitive-data access: include an assertions
entry for user: user:alice on object: employee:diana-record that sets
can_view_sensitive: true so the policy explicitly locks expected admin behavior
and will catch regressions; locate the check block with name "Admin can
terminate employees" and extend its assertions to include the
can_view_sensitive: true assertion alongside can_view, can_edit, and
can_terminate.
In `@stores/issue-tracking/model.fga`:
- Around line 40-42: The can_close permission currently allows only assignee,
lead from assigned_team, or admin from organization to close tickets; if agents
should be able to close any ticket in their organization, update the can_close
definition to include "agent from organization" (i.e., add the symbol agent from
organization to the existing define can_close rule) so agents gain closure
rights across the org; if this was intentional, add documentation in the README
clarifying that can_close excludes general agents and only allows assignees,
team leads, or org admins.
In `@stores/knowledge-base/model.fga`:
- Around line 33-36: The model declares define parent_article: [article] but it
is never used; either remove that unused relation or make it part of the
inheritance chain for article permissions—update the definitions of editor and
viewer (the editor and viewer relations) to include inheritance from
parent_article (e.g., add "or editor or viewer from parent_article" to their
expressions) if you intend article-to-article permission inheritance, otherwise
delete the define parent_article line to keep the model clean.
In `@stores/knowledge-base/model.fga.yaml`:
- Around line 4-72: The model defines an unused relation parent_article on the
article type but no tuples or tests exercise it; either add tuples and tests
that create and assert parent_article behavior (e.g., create
article:parent-child relations and update unit tests to check
inheritance/visibility via parent_article) or remove the parent_article relation
from the model to avoid dead schema. Locate the article type and the
parent_article relation in the model (symbol: parent_article on type article)
and choose one of the two actions: (A) add example tuples linking article:parent
-> article:child and corresponding test cases that assert expected access/role
resolution through parent_article, or (B) remove the parent_article relation
declaration and any references in tests to keep the model and tests consistent.
- Around line 74-167: Add list_objects and list_users test cases to the existing
tests section to cover query perspectives: add a list_objects test named like
"List containers Bob can edit" using user: user:bob and type: container with
assertions for can_edit returning container:engineering-space and
container:api-docs (matching the "Nested container inherits from parent" and
group-based access checks), and add a list_users test named like "List users who
can view getting-started article" using object: article:getting-started with
user_filter type: user and assertions for can_view listing user:alice, user:bob,
and user:charlie (matching the article/container visibility checks); place these
entries alongside the existing check tests in the tests block so they run with
the other scenarios.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 97311e0b-de4f-4b40-8570-da54164f3ee1
📒 Files selected for processing (24)
stores/accounting/README.mdstores/accounting/model.fgastores/accounting/model.fga.yamlstores/applicant-tracking-system/README.mdstores/applicant-tracking-system/model.fgastores/applicant-tracking-system/model.fga.yamlstores/chat/README.mdstores/chat/model.fgastores/chat/model.fga.yamlstores/crm/README.mdstores/crm/model.fgastores/crm/model.fga.yamlstores/file-storage/README.mdstores/file-storage/model.fgastores/file-storage/model.fga.yamlstores/human-resources/README.mdstores/human-resources/model.fgastores/human-resources/model.fga.yamlstores/issue-tracking/README.mdstores/issue-tracking/model.fgastores/issue-tracking/model.fga.yamlstores/knowledge-base/README.mdstores/knowledge-base/model.fgastores/knowledge-base/model.fga.yaml
Member
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
♻️ Duplicate comments (1)
stores/crm/store.fga.yaml (1)
152-159:⚠️ Potential issue | 🟡 MinorRename test to match the assertions.
The case says “only admin can delete”, but owner delete is asserted true at Line 159. Please rename to reflect actual expected behavior.
Suggested rename
- - name: Note owner can edit but only admin can delete + - name: Note owner can edit and delete; manager cannot delete🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stores/crm/store.fga.yaml` around lines 152 - 159, The test title "Note owner can edit but only admin can delete" is inconsistent with the assertions (can_view: true, can_edit: true, can_delete: true) for user: user:charlie on object: note:meeting-notes; update the name field to reflect the actual behavior asserted (e.g., "Note owner can view, edit and delete" or "Note owner can edit and delete") so the test name matches the assertions in the block containing user: user:charlie, object: note:meeting-notes and the assertions can_view/can_edit/can_delete.
🧹 Nitpick comments (17)
stores/knowledge-base/store.fga.yaml (1)
126-146: Isolate intent in “public” and “group-based” tests.These cases use principals that already have other privileges (
user:bobis owner;user:charliemay have contextual access). Consider using a non-owner group member and an org-external user so each test proves exactly one access path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stores/knowledge-base/store.fga.yaml` around lines 126 - 146, The tests "Public article is viewable by anyone" and "Group-based access to containers" use principals with overlapping privileges (user:bob and user:charlie), so isolate intent by replacing those principals with ones that prove a single access path: for the public-article check use an org-external principal (not user:charlie if they have contextual rights) such as user:external_guest against object article:public-faq and assert only can_view:true; for the group-based container check replace user:bob (owner) with a non-owner group member like user:eng_member against object container:engineering-space and assert can_view:true, can_edit:true, can_create_article:true so the test verifies group-based permissions alone.stores/chat/README.md (1)
11-11: Minor wording polish: prefer “provides” over “gives.”This is purely stylistic, but “provides access” reads slightly more formal/consistent for docs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stores/chat/README.md` at line 11, In the README section titled "Group-based channels" replace the phrase "so adding a user to the "Engineering" group automatically gives them access to the `#engineering-general` channel." with a more formal wording such as "so adding a user to the "Engineering" group automatically provides them access to the `#engineering-general` channel." to use "provides" instead of "gives" for consistent documentation tone.stores/applicant-tracking-system/store.fga.yaml (2)
90-97: Test name doesn't reflect all assertions.The test name says "Hiring manager can view and close jobs" but it also asserts
can_edit: true. Consider updating the name to accurately reflect all tested permissions.- - name: Hiring manager can view and close jobs + - name: Hiring manager has full access to assigned jobs🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stores/applicant-tracking-system/store.fga.yaml` around lines 90 - 97, Rename the test "Hiring manager can view and close jobs" to accurately reflect the assertions or remove the extra assertion; specifically update the test name for the block that references user: user:charlie and object: job:swe-senior so it includes "view, edit, and close" (or drop the can_edit: true assertion if edit shouldn't be tested) ensuring the name and the assertions (can_view, can_edit, can_close) stay consistent.
80-200: Consider addinglist_objectsandlist_userstests.The
checkassertions provide good coverage of permission scenarios. However, per coding guidelines, test files should also includelist_objectsandlist_userstests to validate query-based authorization patterns.Example additions:
- name: List jobs viewable by recruiter list_objects: - user: user:bob type: job assertions: can_view: - job:swe-senior - name: List users who can approve offer list_users: - object: offer:diana-offer user_filter: - type: user assertions: can_approve: users: - user:aliceAs per coding guidelines: "Define test cases for OpenFGA models in the tests section of .fga.yaml files, including check, list_objects, and list_users tests."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stores/applicant-tracking-system/store.fga.yaml` around lines 80 - 200, Add missing list-based tests to the tests section to validate query authorization: include a list_objects test where user:bob requests type: job and asserts can_view returns job:swe-senior, and include a list_users test for object: offer:diana-offer with user_filter type: user asserting can_approve returns user:alice; place these alongside the existing check tests (referencing job:swe-senior, offer:diana-offer, user:bob, user:alice) to satisfy the guideline requiring check, list_objects, and list_users entries.stores/issue-tracking/store.fga.yaml (1)
71-179: Consider addinglist_objectsandlist_userstests for comprehensive coverage.The
checktests are well-structured and cover key authorization scenarios. However, per coding guidelines, test suites should also includelist_objectsandlist_userstests to validate the model more thoroughly.Example additions:
- name: List tickets Bob can view list_objects: - user: user:bob type: ticket assertions: - ticket:bug-123 - name: List users who can edit ticket list_users: - object: ticket:bug-123 user_filter: - type: user assertions: can_edit: users: - user:alice - user:bobAs per coding guidelines: "Define test cases for OpenFGA models in the tests section of .fga.yaml files, including check, list_objects, and list_users tests".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stores/issue-tracking/store.fga.yaml` around lines 71 - 179, Add two new tests to the existing tests block to cover list_objects and list_users: (1) a list_objects test such as "List tickets Bob can view" that uses user:user:bob with type: ticket and asserts that ticket:bug-123 appears in the returned list; (2) a list_users test such as "List users who can edit ticket" that targets object: ticket:bug-123 with a user_filter for type: user and asserts can_edit returns user:alice and user:bob. Place both tests alongside the existing check tests (e.g., near "Assignee can view edit and close ticket" and "Admin has full ticket access") so they run as part of the same tests section.stores/healthcare/store.fga.yaml (1)
80-303: Consider addinglist_objectsandlist_userstests.The
checktests are comprehensive and validate all permission scenarios correctly. However, per coding guidelines, test suites should also includelist_objectsandlist_userstests to validate query-based authorization scenarios.Example additions:
- name: List patients accessible by provider list_objects: - user: user:dr-smith type: patient assertions: can_view: - patient:doe - user: user:dr-jones type: patient assertions: can_view: - patient:roe - name: List users who can view patient list_users: - object: patient:doe user_filter: - type: user assertions: can_view: - user:alice - user:dr-smith - user:nurse-williams - user:clerk-brownAs per coding guidelines: "Define test cases for OpenFGA models in the tests section of .fga.yaml files, including check, list_objects, and list_users tests"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stores/healthcare/store.fga.yaml` around lines 80 - 303, Add two new test entries under tests: one "List patients accessible by provider" using list_objects with entries for user:dr-smith and user:dr-jones (type: patient) asserting can_view lists (e.g., patient:doe for dr-smith, patient:roe for dr-jones), and one "List users who can view patient" using list_users with object: patient:doe, user_filter type: user, and assertions listing users (user:alice, user:dr-smith, user:nurse-williams, user:clerk-brown) under can_view; ensure keys are named exactly list_objects and list_users and follow existing test YAML structure like the "Admin has full access to all resources" and "Provider can access their own patients but not others" entries.stores/ecommerce/model.fga (1)
31-31: Consider removing or testing theviewerrelation.The
viewerrelation onproductis defined but never assigned in the test tuples. Based on learnings, unused relations should be removed from the model, or alternatively, add test coverage for direct viewer assignment.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stores/ecommerce/model.fga` at line 31, The model defines a product relation named "viewer" that is unused in test tuples; either remove the unused relation "viewer" from the product definition in model.fga or add test coverage that assigns it. If keeping it, add tuples in your test data that assign the "viewer" relation on specific "product" objects to user principals (so tests exercise product:viewer), and update any permission checks or examples that expect that relation; if removing it, delete the "viewer" relation declaration and ensure no code or tests reference product.viewer.stores/ecommerce/store.fga.yaml (1)
59-267: Consider addinglist_objectsandlist_userstests.The test suite only contains
checkassertions. As per coding guidelines,.fga.yamlfiles should also includelist_objectsandlist_userstests to verify permission enumeration. For example, you could add a test listing all stores user:bob can view or all users who can edit product:sneakers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stores/ecommerce/store.fga.yaml` around lines 59 - 267, Add enumeration tests alongside the existing checks by inserting list_objects and list_users entries into the tests block; for example add a list_objects test for user:user:bob on object_type store (e.g., list stores bob can view such as store:main-shop) and a list_users test for object:product:sneakers to assert which users can_edit or can_view (e.g., expect user:bob, user:charlie, user:diana as appropriate). Ensure the new tests use the same top-level tests list and follow the existing test naming pattern (e.g., "List stores bob can view", "List users who can edit product:sneakers") and reference the same entity identifiers (user:bob, store:main-shop, product:sneakers) so they integrate with the current model assertions.stores/accounting/store.fga.yaml (2)
66-218: Test cases provide good coverage of role-based permissions.The tests thoroughly verify:
- Admin full access including void/post privileges
- Accountant create/edit capabilities with restricted void/approve
- Auditor read-only access pattern
- Member restrictions on sensitive resources
- Workflow-specific permissions (expense approval, purchase order flow, journal entry posting)
Per coding guidelines, consider adding
list_objectsand/orlist_userstests to complement thecheckassertions. For example:📦 Optional list_objects/list_users test examples
- name: List accounts viewable by auditor list_objects: - user: user:charlie type: account assertions: can_view: - account:revenue - name: List users who can edit invoice list_users: - object: invoice:inv-001 user_filter: - type: user assertions: can_edit: users: - user:alice - user:bobAs per coding guidelines: "Define test cases for OpenFGA models in the tests section of .fga.yaml files, including check, list_objects, and list_users tests"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stores/accounting/store.fga.yaml` around lines 66 - 218, Add list_objects and list_users style tests to complement the existing check-based tests: create new test entries similar to "List accounts viewable by auditor" and "List users who can edit invoice" (referencing user: user:charlie, type: account, and object: invoice:inv-001) so you assert which objects a user can list and which users can perform actions via list_objects and list_users; place them alongside the existing test cases in the tests section and mirror the assertion shapes shown in the review (can_view list for account and can_edit users list for invoice).
4-65: Tuples comprehensively cover the role hierarchy and resource relationships.The tuple setup correctly establishes:
- Four distinct users with different organization roles
- Resource-to-organization relationships for all entity types
- Creator/submitter/requester/approver assignments for workflow testing
One gap: no tuple links
invoice:inv-001tocontact:vendor-abc, so theowner from contactview path defined in the model isn't exercised. If you want to validate that contact owners can view related invoices, consider adding:📦 Optional tuple to test contact owner viewing
- user: user:bob relation: creator object: invoice:inv-001 + - user: contact:vendor-abc + relation: contact + object: invoice:inv-001🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stores/accounting/store.fga.yaml` around lines 4 - 65, Add a tuple linking invoice:inv-001 to contact:vendor-abc so the "owner from contact" view path is exercised; specifically, add a tuple similar to the existing resource-to-organization lines but with user: contact:vendor-abc relation: vendor object: invoice:inv-001 (or replace "vendor" with the exact relation name your FGA model uses to attach a contact to an invoice) so that contact owners (e.g., user:diana via contact:vendor-abc owner) can transitively view invoice:inv-001.stores/accounting/model.fga (1)
28-36: Invoice permission model looks correct with contact owner traversal.The
can_viewpermission (line 36) allows contact owners to view invoices viaowner from contact. Note that this traversal requires a tuple linking the invoice to a contact (e.g.,contact:vendor-abc → contact → invoice:inv-001) for the path to be exercised. The test file currently doesn't include such a tuple—consider adding one if you want to validate this access path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stores/accounting/model.fga` around lines 28 - 36, The can_view rule uses the traversal "owner from contact" on the invoice type but tests don't currently create the necessary tuple linking an invoice to a contact; update the test data to add a tuple that connects the invoice to a contact (e.g., set the invoice's contact relation to a contact resource) so the path used by "can_view: owner from contact" can be exercised; locate the invoice model (type invoice, relation define contact) and the tests asserting can_view and add the contact-to-invoice tuple there.stores/call-center/store.fga.yaml (1)
99-117: Add a non-owner agent negative test case for contacts/comments.This section verifies only owned resources. Add at least one resource owned by a different user and assert agent cannot edit/delete it, so over-broad grants are caught by tests.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stores/call-center/store.fga.yaml` around lines 99 - 117, Tests only cover resources owned by charlie (user:charlie) so they miss over-broad grants; add negative test cases by including at least one contact and one comment owned by a different user (e.g., object: contact:jane-doe and object: comment:note-002 or similar) in the same check block and assert can_edit: false and can_delete: false for user: user:charlie so agents cannot modify non-owned contacts/comments; keep existing positive assertions for contact:john-doe and comment:note-001 unchanged.stores/crm/store.fga.yaml (1)
89-97: Test name overstates permissions.Line 89 says “full access”, but
can_deleteis false. Renaming this case would avoid confusion when reading failures.Suggested rename
- - name: Account owner has full access + - name: Account owner can view and edit (not delete)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stores/crm/store.fga.yaml` around lines 89 - 97, The test case name "Account owner has full access" is misleading because assertions for user:charlie on object:account:mega-corp set can_delete: false; update the name field to accurately reflect the granted permissions (e.g., "Account owner has view and edit access only" or "Account owner has view and edit but not delete") so the test name matches the assertions in the block containing user: user:charlie, object: account:mega-corp, and the can_view/can_edit/can_delete assertions.stores/calendar/store.fga.yaml (1)
95-219:can_create_eventis defined in the model but not directly asserted in tests.Add explicit assertions for
calendar:team-calcan_create_event(e.g., admin/scheduler/owner true, viewer/member false) to close a coverage gap.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stores/calendar/store.fga.yaml` around lines 95 - 219, Add explicit assertions for the permission can_create_event on the calendar:team-cal object across the existing scenarios to cover the missing model permission: in the "Scheduler can manage events and links" check add can_create_event: true for user:bob (and for owner/admin user if present), in the "Viewer has read-only access" check add can_create_event: false for user:charlie, and in "Member has no access except as event attendee" add can_create_event: false for user:diana; ensure the new assertions use the same indentation/structure as other assertions under each check for calendar:team-cal so the test harness picks them up.stores/real-estate/model.fga (1)
27-36: Choose a single source of truth for listing tenancy.
listingstores bothorganizationandproperty, but every permission here resolves offorganization, so thepropertyedge is dead for authorization. That leaves two tenant anchors that can drift and push the listing/transaction graph under the wrong org if the tuples ever disagree. Either derive the listing scope frompropertyor remove thepropertyrelation from this sample.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stores/real-estate/model.fga` around lines 27 - 36, The FGA model has two tenant anchors (listing includes both organization and property) which can diverge; pick one source of truth: either derive listing's tenancy from property or remove the property relation entirely. If you choose to derive from property, change the listing definition so authorization resolves via property→organization and update permission rules (can_delete, can_create_transaction, can_close, can_edit, can_view) to consult property’s organization (e.g., resolve listing’s org through property relations before checking org_admin/org_broker/member). If you choose to remove property, delete the define property line and any references and ensure all rules only reference organization (listing→organization) so can_edit/can_view etc. consistently use the single tenant anchor.stores/hospitality/model.fga (1)
42-46: Either use or removereservation#guest.Line 42 defines a
guestedge, but Lines 43-46 never read from it, so the seededreservation:res-001#guest@guest:guest-doetuple currently has no effect on any authorization result. Either thread this relation into acan_*permission or drop it (and the tuple) to keep the sample model focused.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stores/hospitality/model.fga` around lines 42 - 46, The model defines a guest edge (`guest`) and a seeded tuple (`reservation:res-001#guest@guest:guest-doe`) but no permission references it; either remove the `guest` relation and delete the seed tuple, or reference it from a permission (for example include `guest from reservation` in `can_view`/`can_edit`/`can_cancel`) so the seeded guest membership affects authorization; update the `define guest: [guest]` line and corresponding `can_*` definitions (`can_view`, `can_edit`, `can_cancel`, `can_delete`) consistently to reflect your chosen approach.stores/manufacturing/store.fga.yaml (1)
8-10: Splitplant_managerandsupervisorinto separate test identities.Bob currently gets line-derived permissions from both the org-scoped
plant_managerrole and the directsupervisortuple, so a regression in either path can still leave these assertions green. Using different users—or a second production line—would make the suite prove those two permission sources independently.Also applies to: 27-33
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stores/manufacturing/store.fga.yaml` around lines 8 - 10, The test identity currently uses a single principal (user:bob) for both the org-scoped role plant_manager and the direct tuple supervisor, which masks regressions; update the YAML so the plant_manager tuple remains assigned to user:bob (or a dedicated org-level test user) and create a separate principal (e.g., user:carol or user:line_supervisor) for the direct supervisor tuple referenced in the line-scoped assertions; update any related assertions/fixtures that reference the former single principal to use the appropriate new identity names (plant_manager → user:bob, supervisor → user:carol) so the two permission sources are exercised independently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@stores/applicant-tracking-system/store.fga.yaml`:
- Line 1: The YAML test model name string "Application Tracking System
Authorization Model Tests" is inconsistent with the folder name
`applicant-tracking-system`; update the name value in the file (the YAML
top-level key `name`) to match the folder by changing it to "Applicant Tracking
System Authorization Model Tests" (or alternatively rename the folder to
`application-tracking-system`) so the `name` and folder are aligned.
In `@stores/calendar/store.fga.yaml`:
- Around line 59-219: Add reverse-query tests under the same tests section by
adding list_objects and list_users cases that exercise the calendar model's
reverse queries: include list_objects entries for users (user:alice, user:bob,
user:charlie, user:diana) returning the expected objects (calendar:team-cal,
event:standup, link:booking-page, recording:standup-rec, webinar:product-launch)
according to their permissions, and include list_users entries for key objects
(calendar:team-cal, event:standup, link:booking-page, recording:standup-rec,
webinar:product-launch) returning the expected users (alice, bob, charlie,
diana) that can perform specific actions (e.g., view/edit/publish) so the test
suite validates both check and reverse-query semantics for the calendar model.
In `@stores/call-center/model.fga`:
- Line 31: can_edit is too permissive because "agent from organization" lets any
org agent edit any contact; change the rule so agents can only edit contacts
they own. Update the define can_edit rule (currently: "define can_edit: owner or
agent from organization or can_delete") to require the agent to be the contact
owner (e.g., "define can_edit: owner or (agent from organization and owner)" or
the equivalent FGA relation that enforces agent == owner) while keeping
can_delete unchanged so only owners or agents who are also the contact owner can
edit.
In `@stores/call-center/store.fga.yaml`:
- Around line 48-174: Add list_objects and list_users test cases to the existing
tests block so permission-discovery is covered: for each role/actor (user:alice,
user:bob, user:charlie, user:diana) add list_objects checks that assert which
object IDs they should see for each relation (e.g., can_view on call:call-001,
recording:rec-001, contact:john-doe, comment:note-001) and add list_users checks
for resources where user enumeration matters (e.g., comment:note-001 list_users
for can_edit/can_delete should return user:charlie as author and
user:alice/supervisor where applicable); mirror the expectations already
expressed in the check entries (Admin sees all, Supervisor sees recordings and
comments per Supervisor can view/manage, Agent sees own comments/contacts but
not recordings, Member sees none) and name the tests clearly (e.g., "List
objects for Admin", "List users for Comment author") so they live alongside the
existing check tests.
In `@stores/chat/store.fga.yaml`:
- Around line 66-182: Add list_objects and list_users cases to the tests section
so discovery and reverse-user resolution are covered: for each existing check
block (e.g., "Channel owner has full access", "Group members can view and post
in channel", "DM participants can view and post", "Message sender can edit own
messages", "DM message visibility"), add corresponding list_objects entries
asserting which conversation or message IDs (conversation:engineering-general,
conversation:bob-charlie-dm, message:msg-001, message:dm-001) are returned for
the relevant users (user:bob, user:charlie, user:alice, user:diana) and add
list_users entries for objects where applicable to assert which users are
discoverable for that object; ensure keys are named exactly list_objects and
list_users and that assertions match the expected membership/visibility in the
existing check assertions.
In `@stores/crm/store.fga.yaml`:
- Around line 64-180: Add list_objects and list_users tests alongside existing
check tests to validate reverse queries for key relations (e.g., can_view on
account and opportunity). For example, create list_objects tests that query
which accounts and opportunities user:alice, user:bob, and user:charlie can_view
(reference existing check cases like "Admin has full CRM access", "Sales manager
can view and close opportunities", and "Opportunity follows account ownership")
and create list_users tests that query which users can_view account:mega-corp
and opportunity:big-deal. Ensure test names are descriptive (e.g., "List objects
viewable by users" and "List users who can_view account/opportunity") and mirror
the assertions in the corresponding check tests so reverse-query behavior is
covered.
In `@stores/ecommerce/store.fga.yaml`:
- Around line 256-267: Rename the test titled "Non-store member cannot see
orders" to reflect the actual checks being performed against customer:diana and
product:sneakers (e.g., "Non-privileged users cannot access resources outside
their scope"); update the name string so it references the specific scope being
validated (user:charlie and user:diana attempting access to customer:diana and
product:sneakers with assertions can_delete/can_view/can_create_review/can_edit)
to keep the test title consistent with the assertions.
- Around line 51-57: The order tuple for order:ord-001 is missing the customer
relation required by the model paths used by can_cancel and can_view; add a
relation tuple linking the customer (e.g., user:diana or the intended customer
principal) to order:ord-001 with relation "customer" so the
account_owner-from-customer path is exercised — update the tuples alongside the
existing lines for order:ord-001 to include the missing "customer" relation.
In `@stores/hospitality/store.fga.yaml`:
- Around line 79-313: Add representative list_objects and list_users tests to
the existing tests suite: create list_objects entries that assert which object
IDs each key user (user:alice, user:bob, user:charlie, user:maria, user:diana,
user:eve, user:frank) can see under organization:grand-hotel-group and
hotel:grand-plaza (covering hotels, rooms like room:room-101, reservations like
reservation:res-001 and services like service:spa-treatment) to validate
inherited read permissions; add list_users entries for core objects
(hotel:grand-plaza, room:room-101, reservation:res-001, service:spa-treatment)
asserting which users appear for viewer and editor relations (can_view,
can_edit, can_view_revenue) to validate management visibility; ensure cases
include negative expectations (users who should not be listed, e.g., maria for
other hotels) and mirror the check assertions so listing behavior matches
inherited organization->hotel->child permissions.
In `@stores/knowledge-base/store.fga.yaml`:
- Around line 65-158: The tests block currently only contains check tests; add
corresponding list_objects and list_users tests to validate reverse queries for
key permissions (e.g., can_view) so regressions are caught. Specifically, for
users like user:alice, user:bob, user:charlie add list_objects entries asserting
which containers/articles/attachments they can list for can_view (e.g.,
container:engineering-space, container:api-docs, article:getting-started,
article:public-faq, attachment:diagram-png), and add list_users entries for
those objects asserting which users should be returned for can_view (e.g.,
container:engineering-space → alice/bob/charlie as appropriate,
article:public-faq → anyone/bob/charlie, attachment:diagram-png → bob/charlie).
Ensure the new tests follow the same YAML test shape and use the existing object
IDs and permission keys (list_objects, list_users, check, user:, object:,
assertions) so they integrate with the repository’s required test schema.
In `@stores/lms/store.fga.yaml`:
- Around line 71-262: The tests section only contains "check" cases (e.g., test
names "Admin has full access to all resources", "Instructor can manage courses
and grade activities", etc.) but is missing required list_objects and list_users
test blocks; add list_objects tests that assert which objects each principal
(user:alice, user:bob, user:charlie, user:diana) can list for each resource type
(course:intro-cs, class:section-a, content:lecture-slides,
collection:cs-fundamentals, activity:homework-1) and add list_users tests that
assert which users are returned for given objects (e.g., who can_view, can_edit,
can_grade, can_submit each object) so the suite covers check, list_objects, and
list_users as required by the policy; keep test names consistent with existing
ones and reuse the same principals and objects to ensure coverage.
In `@stores/manufacturing/store.fga.yaml`:
- Around line 77-353: The test suite is missing list_objects and list_users
blocks to validate reverse-expansion; add new tests named e.g. "list_objects:
Admin sees all resources" and "list_users: Who can_edit a machine" (and a couple
representative ones for work_order approvals and organization creation) that
exercise list_objects for resources like production_line:line-alpha,
machine:cnc-001, work_order:wo-001 and list_users for permissions like can_edit,
can_approve, can_create_part using users alice, bob, charlie to mirror key
permission paths; place these new entries alongside the existing check tests,
include expected membership assertions (objects returned for list_objects and
users returned for list_users) to cover admin, plant manager and engineer
scenarios without duplicating every check case.
In `@stores/payment/store.fga.yaml`:
- Around line 212-219: The test "Payment creator can view but not edit or
refund" is inconsistent: assertions for the case with user:bob on object
payment:txn-001 currently set can_edit: true and can_refund: true while the name
says they should be false, and user:bob also has finance_manager role plus
creator relation. Fix by either (a) changing the assertions in that test to
can_edit: false and can_refund: false to match the test name, (b) renaming the
test to reflect that a finance_manager-creator can edit/refund, or (c) add a new
test using a dedicated user who is only the creator (no finance_manager role) to
assert can_view: true, can_edit: false, can_refund: false; update the test entry
referencing user:bob, payment:txn-001 and any role assignments accordingly.
In `@stores/real-estate/store.fga.yaml`:
- Around line 72-303: The tests section currently only defines check tests and
is missing list_objects and list_users coverage; add representative list_objects
and list_users test cases alongside the existing checks to exercise reverse
queries (e.g., for users user:charlie, user:eve, user:bob and objects
listing:listing-001, listing-002, transaction:txn-001, property:prop-123,
inspection:insp-001) so agent isolation and transaction visibility are
validated; create list_objects entries that query which objects a user can
perform core actions on (view, edit, create_transaction) and list_users entries
that query which users can act on a given object (listing:listing-001,
transaction:txn-001) following the same assertions style used in the check
blocks.
---
Duplicate comments:
In `@stores/crm/store.fga.yaml`:
- Around line 152-159: The test title "Note owner can edit but only admin can
delete" is inconsistent with the assertions (can_view: true, can_edit: true,
can_delete: true) for user: user:charlie on object: note:meeting-notes; update
the name field to reflect the actual behavior asserted (e.g., "Note owner can
view, edit and delete" or "Note owner can edit and delete") so the test name
matches the assertions in the block containing user: user:charlie, object:
note:meeting-notes and the assertions can_view/can_edit/can_delete.
---
Nitpick comments:
In `@stores/accounting/model.fga`:
- Around line 28-36: The can_view rule uses the traversal "owner from contact"
on the invoice type but tests don't currently create the necessary tuple linking
an invoice to a contact; update the test data to add a tuple that connects the
invoice to a contact (e.g., set the invoice's contact relation to a contact
resource) so the path used by "can_view: owner from contact" can be exercised;
locate the invoice model (type invoice, relation define contact) and the tests
asserting can_view and add the contact-to-invoice tuple there.
In `@stores/accounting/store.fga.yaml`:
- Around line 66-218: Add list_objects and list_users style tests to complement
the existing check-based tests: create new test entries similar to "List
accounts viewable by auditor" and "List users who can edit invoice" (referencing
user: user:charlie, type: account, and object: invoice:inv-001) so you assert
which objects a user can list and which users can perform actions via
list_objects and list_users; place them alongside the existing test cases in the
tests section and mirror the assertion shapes shown in the review (can_view list
for account and can_edit users list for invoice).
- Around line 4-65: Add a tuple linking invoice:inv-001 to contact:vendor-abc so
the "owner from contact" view path is exercised; specifically, add a tuple
similar to the existing resource-to-organization lines but with user:
contact:vendor-abc relation: vendor object: invoice:inv-001 (or replace "vendor"
with the exact relation name your FGA model uses to attach a contact to an
invoice) so that contact owners (e.g., user:diana via contact:vendor-abc owner)
can transitively view invoice:inv-001.
In `@stores/applicant-tracking-system/store.fga.yaml`:
- Around line 90-97: Rename the test "Hiring manager can view and close jobs" to
accurately reflect the assertions or remove the extra assertion; specifically
update the test name for the block that references user: user:charlie and
object: job:swe-senior so it includes "view, edit, and close" (or drop the
can_edit: true assertion if edit shouldn't be tested) ensuring the name and the
assertions (can_view, can_edit, can_close) stay consistent.
- Around line 80-200: Add missing list-based tests to the tests section to
validate query authorization: include a list_objects test where user:bob
requests type: job and asserts can_view returns job:swe-senior, and include a
list_users test for object: offer:diana-offer with user_filter type: user
asserting can_approve returns user:alice; place these alongside the existing
check tests (referencing job:swe-senior, offer:diana-offer, user:bob,
user:alice) to satisfy the guideline requiring check, list_objects, and
list_users entries.
In `@stores/calendar/store.fga.yaml`:
- Around line 95-219: Add explicit assertions for the permission
can_create_event on the calendar:team-cal object across the existing scenarios
to cover the missing model permission: in the "Scheduler can manage events and
links" check add can_create_event: true for user:bob (and for owner/admin user
if present), in the "Viewer has read-only access" check add can_create_event:
false for user:charlie, and in "Member has no access except as event attendee"
add can_create_event: false for user:diana; ensure the new assertions use the
same indentation/structure as other assertions under each check for
calendar:team-cal so the test harness picks them up.
In `@stores/call-center/store.fga.yaml`:
- Around line 99-117: Tests only cover resources owned by charlie (user:charlie)
so they miss over-broad grants; add negative test cases by including at least
one contact and one comment owned by a different user (e.g., object:
contact:jane-doe and object: comment:note-002 or similar) in the same check
block and assert can_edit: false and can_delete: false for user: user:charlie so
agents cannot modify non-owned contacts/comments; keep existing positive
assertions for contact:john-doe and comment:note-001 unchanged.
In `@stores/chat/README.md`:
- Line 11: In the README section titled "Group-based channels" replace the
phrase "so adding a user to the "Engineering" group automatically gives them
access to the `#engineering-general` channel." with a more formal wording such
as "so adding a user to the "Engineering" group automatically provides them
access to the `#engineering-general` channel." to use "provides" instead of
"gives" for consistent documentation tone.
In `@stores/crm/store.fga.yaml`:
- Around line 89-97: The test case name "Account owner has full access" is
misleading because assertions for user:charlie on object:account:mega-corp set
can_delete: false; update the name field to accurately reflect the granted
permissions (e.g., "Account owner has view and edit access only" or "Account
owner has view and edit but not delete") so the test name matches the assertions
in the block containing user: user:charlie, object: account:mega-corp, and the
can_view/can_edit/can_delete assertions.
In `@stores/ecommerce/model.fga`:
- Line 31: The model defines a product relation named "viewer" that is unused in
test tuples; either remove the unused relation "viewer" from the product
definition in model.fga or add test coverage that assigns it. If keeping it, add
tuples in your test data that assign the "viewer" relation on specific "product"
objects to user principals (so tests exercise product:viewer), and update any
permission checks or examples that expect that relation; if removing it, delete
the "viewer" relation declaration and ensure no code or tests reference
product.viewer.
In `@stores/ecommerce/store.fga.yaml`:
- Around line 59-267: Add enumeration tests alongside the existing checks by
inserting list_objects and list_users entries into the tests block; for example
add a list_objects test for user:user:bob on object_type store (e.g., list
stores bob can view such as store:main-shop) and a list_users test for
object:product:sneakers to assert which users can_edit or can_view (e.g., expect
user:bob, user:charlie, user:diana as appropriate). Ensure the new tests use the
same top-level tests list and follow the existing test naming pattern (e.g.,
"List stores bob can view", "List users who can edit product:sneakers") and
reference the same entity identifiers (user:bob, store:main-shop,
product:sneakers) so they integrate with the current model assertions.
In `@stores/healthcare/store.fga.yaml`:
- Around line 80-303: Add two new test entries under tests: one "List patients
accessible by provider" using list_objects with entries for user:dr-smith and
user:dr-jones (type: patient) asserting can_view lists (e.g., patient:doe for
dr-smith, patient:roe for dr-jones), and one "List users who can view patient"
using list_users with object: patient:doe, user_filter type: user, and
assertions listing users (user:alice, user:dr-smith, user:nurse-williams,
user:clerk-brown) under can_view; ensure keys are named exactly list_objects and
list_users and follow existing test YAML structure like the "Admin has full
access to all resources" and "Provider can access their own patients but not
others" entries.
In `@stores/hospitality/model.fga`:
- Around line 42-46: The model defines a guest edge (`guest`) and a seeded tuple
(`reservation:res-001#guest@guest:guest-doe`) but no permission references it;
either remove the `guest` relation and delete the seed tuple, or reference it
from a permission (for example include `guest from reservation` in
`can_view`/`can_edit`/`can_cancel`) so the seeded guest membership affects
authorization; update the `define guest: [guest]` line and corresponding `can_*`
definitions (`can_view`, `can_edit`, `can_cancel`, `can_delete`) consistently to
reflect your chosen approach.
In `@stores/issue-tracking/store.fga.yaml`:
- Around line 71-179: Add two new tests to the existing tests block to cover
list_objects and list_users: (1) a list_objects test such as "List tickets Bob
can view" that uses user:user:bob with type: ticket and asserts that
ticket:bug-123 appears in the returned list; (2) a list_users test such as "List
users who can edit ticket" that targets object: ticket:bug-123 with a
user_filter for type: user and asserts can_edit returns user:alice and user:bob.
Place both tests alongside the existing check tests (e.g., near "Assignee can
view edit and close ticket" and "Admin has full ticket access") so they run as
part of the same tests section.
In `@stores/knowledge-base/store.fga.yaml`:
- Around line 126-146: The tests "Public article is viewable by anyone" and
"Group-based access to containers" use principals with overlapping privileges
(user:bob and user:charlie), so isolate intent by replacing those principals
with ones that prove a single access path: for the public-article check use an
org-external principal (not user:charlie if they have contextual rights) such as
user:external_guest against object article:public-faq and assert only
can_view:true; for the group-based container check replace user:bob (owner) with
a non-owner group member like user:eng_member against object
container:engineering-space and assert can_view:true, can_edit:true,
can_create_article:true so the test verifies group-based permissions alone.
In `@stores/manufacturing/store.fga.yaml`:
- Around line 8-10: The test identity currently uses a single principal
(user:bob) for both the org-scoped role plant_manager and the direct tuple
supervisor, which masks regressions; update the YAML so the plant_manager tuple
remains assigned to user:bob (or a dedicated org-level test user) and create a
separate principal (e.g., user:carol or user:line_supervisor) for the direct
supervisor tuple referenced in the line-scoped assertions; update any related
assertions/fixtures that reference the former single principal to use the
appropriate new identity names (plant_manager → user:bob, supervisor →
user:carol) so the two permission sources are exercised independently.
In `@stores/real-estate/model.fga`:
- Around line 27-36: The FGA model has two tenant anchors (listing includes both
organization and property) which can diverge; pick one source of truth: either
derive listing's tenancy from property or remove the property relation entirely.
If you choose to derive from property, change the listing definition so
authorization resolves via property→organization and update permission rules
(can_delete, can_create_transaction, can_close, can_edit, can_view) to consult
property’s organization (e.g., resolve listing’s org through property relations
before checking org_admin/org_broker/member). If you choose to remove property,
delete the define property line and any references and ensure all rules only
reference organization (listing→organization) so can_edit/can_view etc.
consistently use the single tenant anchor.
🪄 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
Run ID: 1be7e17f-10d8-4529-b6eb-b92201ed5a19
📒 Files selected for processing (57)
stores/accounting/README.mdstores/accounting/model.fgastores/accounting/store.fga.yamlstores/ads/README.mdstores/ads/model.fgastores/ads/store.fga.yamlstores/applicant-tracking-system/README.mdstores/applicant-tracking-system/model.fgastores/applicant-tracking-system/store.fga.yamlstores/calendar/README.mdstores/calendar/model.fgastores/calendar/store.fga.yamlstores/call-center/README.mdstores/call-center/model.fgastores/call-center/store.fga.yamlstores/chat/README.mdstores/chat/model.fgastores/chat/store.fga.yamlstores/crm/README.mdstores/crm/model.fgastores/crm/store.fga.yamlstores/ecommerce/README.mdstores/ecommerce/model.fgastores/ecommerce/store.fga.yamlstores/file-storage/README.mdstores/file-storage/model.fgastores/file-storage/store.fga.yamlstores/healthcare/README.mdstores/healthcare/model.fgastores/healthcare/store.fga.yamlstores/hospitality/README.mdstores/hospitality/model.fgastores/hospitality/store.fga.yamlstores/human-resources/README.mdstores/human-resources/model.fgastores/human-resources/store.fga.yamlstores/issue-tracking/README.mdstores/issue-tracking/model.fgastores/issue-tracking/store.fga.yamlstores/kms/README.mdstores/kms/model.fgastores/kms/store.fga.yamlstores/knowledge-base/README.mdstores/knowledge-base/model.fgastores/knowledge-base/store.fga.yamlstores/lms/README.mdstores/lms/model.fgastores/lms/store.fga.yamlstores/manufacturing/README.mdstores/manufacturing/model.fgastores/manufacturing/store.fga.yamlstores/payment/README.mdstores/payment/model.fgastores/payment/store.fga.yamlstores/real-estate/README.mdstores/real-estate/model.fgastores/real-estate/store.fga.yaml
✅ Files skipped from review due to trivial changes (24)
- stores/kms/README.md
- stores/payment/README.md
- stores/real-estate/README.md
- stores/lms/README.md
- stores/accounting/README.md
- stores/calendar/README.md
- stores/call-center/README.md
- stores/crm/README.md
- stores/ecommerce/README.md
- stores/manufacturing/README.md
- stores/ads/README.md
- stores/applicant-tracking-system/README.md
- stores/healthcare/README.md
- stores/hospitality/README.md
- stores/human-resources/README.md
- stores/file-storage/README.md
- stores/knowledge-base/README.md
- stores/applicant-tracking-system/model.fga
- stores/issue-tracking/README.md
- stores/issue-tracking/model.fga
- stores/lms/model.fga
- stores/payment/model.fga
- stores/knowledge-base/model.fga
- stores/file-storage/model.fga
🚧 Files skipped from review as they are similar to previous changes (3)
- stores/human-resources/model.fga
- stores/chat/model.fga
- stores/crm/model.fga
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Member
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
stores/manufacturing/store.fga.yaml (1)
355-432: Add permission-based list assertions to improve test coverage.The current list test cases assert only direct relations (
supervisor,assigned_operator,creator,assignee,inspector) and internal helper relations (org_admin,org_engineer,org_plant_manager). The verification script confirms that 9 computed relations defined in the model are not exercised by these assertions:can_approve,can_create_machine,can_create_part,can_create_quality_report,can_create_supplier,can_delete,can_edit,can_view, andmember.Consider supplementing or replacing some of the direct-relation tests with assertions for these effective permissions to strengthen coverage:
Example refactoring
- name: List users with work order relationships list_users: - object: work_order:wo-001 user_filter: - type: user assertions: creator: users: - user:bob assignee: users: - user:eve + can_approve: + users: + - user:alice + - user:bob + + - name: Users can list machines by effective permissions + list_objects: + - user: user:eve + type: machine + assertions: + can_edit: + - machine:cnc-001 + can_view: + - machine:cnc-001 + - machine:press-002🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stores/manufacturing/store.fga.yaml` around lines 355 - 432, The test YAML only asserts direct relations (supervisor, assigned_operator, creator, assignee, inspector) and a few internal roles, but misses coverage for computed permission relations; update the existing list_objects and list_users blocks (e.g., the "Users can list objects they have roles in", "Admin can list all production and organizational resources", and the three "List users..." sections) to include assertions for the computed relations can_approve, can_create_machine, can_create_part, can_create_quality_report, can_create_supplier, can_delete, can_edit, can_view, and member against the relevant users (user:alice, user:bob, user:charlie, user:diana, user:eve) and objects (production_line:line-alpha, machine:cnc-001, work_order:wo-001, quality_report:qr-001); either add new assertion blocks for these permission names or replace some direct-relation assertions with equivalent permission assertions so each of the nine computed relations is exercised.stores/crm/store.fga.yaml (1)
182-199: Rename this case or switch it to permission assertions.
owneronly verifies the raw tuples foraccount:mega-corp,contact:john-doe, andopportunity:big-deal. It does not validate the access behavior implied by "charlie can access", socan_view/can_editregressions on these objects would still pass this block.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stores/crm/store.fga.yaml` around lines 182 - 199, The test case under list_objects that uses assertions.owner for user: user:charlie only checks raw tuples and not actual access policies; either rename the case to indicate it's validating tuple presence (e.g., "List objects owned by charlie") or replace assertions.owner with permission assertions such as assertions.can_view or assertions.can_edit for the types account, contact, and opportunity so the suite validates actual access behavior for user: user:charlie instead of just tuple membership; update the three entries (type: account/contact/opportunity) accordingly within the same list_objects block.stores/lms/store.fga.yaml (1)
264-350: Thelist_*coverage here is tuple-oriented, not permission-oriented.These cases only assert raw relations (
enrolled_instructor,student,creator,owner, etc.), so a reverse-query regression incan_view,can_edit,can_grade, orcan_submitwould still pass. The collection entry under "Admin can list all organization resources" also switches touser:bob, so admin coverage for collections is not actually being tested.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stores/lms/store.fga.yaml` around lines 264 - 350, The current tests only assert raw relations (enrolled_instructor, student, creator, owner, etc.) so they miss permission-oriented regressions; update the scenarios to assert permission-based relations (e.g., can_view, can_edit, can_grade, can_submit) in the relevant list_objects/list_users blocks (for course, class, activity) so reverse-query permission failures are caught, and fix the "Admin can list all organization resources" case to use user:alice for the collection owner/admin assertion (not user:bob) to ensure admin collection coverage is actually tested.stores/healthcare/store.fga.yaml (1)
305-375: Add at least one permission-basedlist_*case for patient access.Everything in this block enumerates direct relations (
primary_provider,care_team,attending_provider, etc.), so reverse-query behavior forcan_view,can_edit, andcan_view_sensitiveonpatient:doe/encounter:enc-001is still untested.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stores/healthcare/store.fga.yaml` around lines 305 - 375, Add a permission-based reverse list test for patient access: add a new list_users or list_objects case that queries patient:doe by permission (can_view, can_edit, or can_view_sensitive) rather than by direct relation; for example, create a list_users entry with object: patient:doe, user_filter: - type: user and assertions that list users under can_view (and/or can_edit, can_view_sensitive) showing expected users (e.g., user:dr-smith, user:nurse-williams as appropriate). Target the existing test blocks around List users with primary provider access and List users with care team access so the new case exercises permission-based reverse queries for patient:doe (and add a similar case for encounter:enc-001 if desired for attending_provider).README.md (1)
10-12: Consider matching TOC order to section order for easier scanning.The TOC lists OpenFGA Features before Authorization Patterns, but the document presents Authorization Patterns first. Keeping these aligned improves navigation flow.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 10 - 12, The table of contents order doesn't match the document body: swap the TOC entries so "Authorization Patterns" appears before "OpenFGA Features" (keep "Industry Examples" after them) to mirror the actual section order; update the TOC lines referencing "Authorization Patterns", "OpenFGA Features", and "Industry Examples" so their sequence matches the document sections.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/copilot-instructions.md:
- Line 28: The sentence "Test every model change by running the tests`fga model
test --tests store.fga.yaml`." is missing a space before the inline code; update
the text so there is a space between "tests" and the inline command (i.e.,
"tests `fga model test --tests store.fga.yaml`") to correct the formatting and
improve readability.
- Around line 32-34: The fenced code block containing the command "npx skills
add openfga/agent-skills" is missing a language identifier which violates
markdownlint MD040; update the fence to include a shell language tag (e.g.,
"sh") by changing the opening triple backticks to "```sh" so the block reads as
a shell command block.
In `@CLAUDE.md`:
- Around line 5-7: The fenced code block containing the shell command "npx
skills add openfga/agent-skills" is missing a language identifier and triggers
markdownlint MD040; update the block delimiter from ``` to ```sh (or ```bash) so
it becomes a shell-highlighted code block (e.g., replace the opening ``` before
the line `npx skills add openfga/agent-skills` with ```sh).
---
Nitpick comments:
In `@README.md`:
- Around line 10-12: The table of contents order doesn't match the document
body: swap the TOC entries so "Authorization Patterns" appears before "OpenFGA
Features" (keep "Industry Examples" after them) to mirror the actual section
order; update the TOC lines referencing "Authorization Patterns", "OpenFGA
Features", and "Industry Examples" so their sequence matches the document
sections.
In `@stores/crm/store.fga.yaml`:
- Around line 182-199: The test case under list_objects that uses
assertions.owner for user: user:charlie only checks raw tuples and not actual
access policies; either rename the case to indicate it's validating tuple
presence (e.g., "List objects owned by charlie") or replace assertions.owner
with permission assertions such as assertions.can_view or assertions.can_edit
for the types account, contact, and opportunity so the suite validates actual
access behavior for user: user:charlie instead of just tuple membership; update
the three entries (type: account/contact/opportunity) accordingly within the
same list_objects block.
In `@stores/healthcare/store.fga.yaml`:
- Around line 305-375: Add a permission-based reverse list test for patient
access: add a new list_users or list_objects case that queries patient:doe by
permission (can_view, can_edit, or can_view_sensitive) rather than by direct
relation; for example, create a list_users entry with object: patient:doe,
user_filter: - type: user and assertions that list users under can_view (and/or
can_edit, can_view_sensitive) showing expected users (e.g., user:dr-smith,
user:nurse-williams as appropriate). Target the existing test blocks around List
users with primary provider access and List users with care team access so the
new case exercises permission-based reverse queries for patient:doe (and add a
similar case for encounter:enc-001 if desired for attending_provider).
In `@stores/lms/store.fga.yaml`:
- Around line 264-350: The current tests only assert raw relations
(enrolled_instructor, student, creator, owner, etc.) so they miss
permission-oriented regressions; update the scenarios to assert permission-based
relations (e.g., can_view, can_edit, can_grade, can_submit) in the relevant
list_objects/list_users blocks (for course, class, activity) so reverse-query
permission failures are caught, and fix the "Admin can list all organization
resources" case to use user:alice for the collection owner/admin assertion (not
user:bob) to ensure admin collection coverage is actually tested.
In `@stores/manufacturing/store.fga.yaml`:
- Around line 355-432: The test YAML only asserts direct relations (supervisor,
assigned_operator, creator, assignee, inspector) and a few internal roles, but
misses coverage for computed permission relations; update the existing
list_objects and list_users blocks (e.g., the "Users can list objects they have
roles in", "Admin can list all production and organizational resources", and the
three "List users..." sections) to include assertions for the computed relations
can_approve, can_create_machine, can_create_part, can_create_quality_report,
can_create_supplier, can_delete, can_edit, can_view, and member against the
relevant users (user:alice, user:bob, user:charlie, user:diana, user:eve) and
objects (production_line:line-alpha, machine:cnc-001, work_order:wo-001,
quality_report:qr-001); either add new assertion blocks for these permission
names or replace some direct-relation assertions with equivalent permission
assertions so each of the nine computed relations is exercised.
🪄 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
Run ID: 48532a7d-a611-4a77-9b27-79f1771a15bb
📒 Files selected for processing (25)
.github/copilot-instructions.mdCLAUDE.mdREADME.mdstores/accounting/store.fga.yamlstores/ads/store.fga.yamlstores/applicant-tracking-system/store.fga.yamlstores/calendar/store.fga.yamlstores/call-center/store.fga.yamlstores/chat/store.fga.yamlstores/crm/store.fga.yamlstores/ecommerce/store.fga.yamlstores/file-storage/store.fga.yamlstores/healthcare/README.mdstores/healthcare/store.fga.yamlstores/hospitality/store.fga.yamlstores/human-resources/README.mdstores/human-resources/store.fga.yamlstores/issue-tracking/store.fga.yamlstores/kms/store.fga.yamlstores/knowledge-base/store.fga.yamlstores/lms/store.fga.yamlstores/manufacturing/store.fga.yamlstores/payment/store.fga.yamlstores/real-estate/README.mdstores/real-estate/store.fga.yaml
✅ Files skipped from review due to trivial changes (6)
- stores/human-resources/README.md
- stores/accounting/store.fga.yaml
- stores/human-resources/store.fga.yaml
- stores/hospitality/store.fga.yaml
- stores/healthcare/README.md
- stores/real-estate/README.md
🚧 Files skipped from review as they are similar to previous changes (7)
- stores/file-storage/store.fga.yaml
- stores/chat/store.fga.yaml
- stores/kms/store.fga.yaml
- stores/ecommerce/store.fga.yaml
- stores/payment/store.fga.yaml
- stores/calendar/store.fga.yaml
- stores/applicant-tracking-system/store.fga.yaml
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
dyeam0
reviewed
Mar 30, 2026
Member
dyeam0
left a comment
dyeam0
left a comment
There was a problem hiding this comment.
Using AI, it caught some things across the models:
- ATS
application.can_viewdead clause - stores/applicant-tracking-system/model.fga:56: define can_view: can_view from job or candidate. The candidate relation stores candidate type objects, not user objects, so or candidate never matches a user check. This is dead code that implies candidates can view their own applications but doesn't actually do that. - Inconsistent
model_filepaths - healthcare, hospitality, manufacturing, and real-estate use model_file: model.fga while the other 15 use model_file: ./model.fga.
(more comments coming related to the README...)
Member
Author
|
@dyeam0 addressed those and Copilot's ones |
dyeam0
approved these changes
Apr 1, 2026
Member
dyeam0
left a comment
dyeam0
left a comment
There was a problem hiding this comment.
I'm paraphrasing some of this from AI's review. Regarding README format:
- Option A: Align new READMEs with existing format. Add Scenario, Expected Outcomes, and inline model sections to each new README.
- Option B: Keep the new format, acknowledge the inconsistency, fix later. The new READMEs are functional. They explain what each model does and how to run it. The existing READMEs can be migrated to a new format (or new ones enriched) in a follow-up PR.
Even AI's recommendation is Option B because:
- The model code and tests are solid and passing
- The format inconsistency can be a follow-up issue
Member
Author
|
Thanks @dyeam0 ! |
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
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.
Description
What problem is being solved?
How is it being solved?
What changes are made to solve it?
References
Review Checklist
mainSummary by CodeRabbit