feat: migrate agents-access to org-scoped system role for proper chat RBAC#24438
feat: migrate agents-access to org-scoped system role for proper chat RBAC#24438
Conversation
johnstcn
force-pushed
the
cian/agents-access-org-role
branch
2 times, most recently
from
075f78c to
2848396
Compare
|
/coder-agents-review |
![coder-agents-review[bot]](https://avatars.githubusercontent.com/in/3454270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Well-structured RBAC migration. The single-gate model (agents-access as sole chat permission source) is cleaner than the previous split where org members had implicit chat CRUD. The handler auth improvements (ActionUpdate on chat.RBACObject() instead of ActionCreate on synthetic resources) are correct. The migration is DML-only, idempotent, and production-safe. The ChatUsageCRU/ChatUsageDelete test split precisely documents the new permission boundary.
Pariston on the design: "I tried to build a case against this and came up short. The problem is correctly understood. The solution is proportional. The fix is at the right causal level: the role definition, not the symptom layer."
1 P2, 5 P3, 4 Nit. The P2 is a cross-org cost data leak via AnyOrganization() that merits attention.
🤖 This review was automatically generated with Coder Agents.
johnstcn
force-pushed
the
cian/agents-access-org-role
branch
from
87b92c3 to
26205a6
Compare
|
/coder-agents-review |
There was a problem hiding this comment.
All 10 posted findings from R1 are resolved: 8 fixed in code, 1 accepted by human reviewer (DEREM-3, cross-org cost queries, with TODO documenting the limitation), 1 deferred with ticket (DEREM-4, CODAGT-161). R2 panel (Bisky, Mafuuu, Kite, Razor) and Netero found no new issues.
Mafuuu on contract fidelity: "Members without agents-access have zero chat access. Tests verify: list returns 0 chats (SQL auth filter), get returns 404 (dbauthz wraps as not-found), update returns 404, message send returns 404."
🤖 This review was automatically generated with Coder Agents.
There was a problem hiding this comment.
R3 re-review after rebase. No new findings from Netero or the panel (Mafuuu, Kite, Razor). The delta since R2 is a merge from main bringing string constant extraction and unrelated changes; no authorization paths were modified. All R1 findings remain resolved. CI passing (11/20 pending at review time, 9 skipped).
🤖 This review was automatically generated with Coder Agents.
… RBAC The agents-access role previously granted chat permissions at user scope, but chats are org-scoped objects. Rego skips user-level perms when org_owner is set, making the grants invisible. Handler-level band-aids used synthetic non-org-scoped objects as a workaround. This commit: - Migrates agents-access from users.rbac_roles (site-level) to organization_members.roles (org-scoped) via DB migration - Redefines agents-access as a predefined org-scoped builtin role alongside organization-admin, organization-auditor, etc., with Member permissions granting chat create/read/update - Excludes ResourceChat from OrgMemberPermissions so org membership alone no longer grants chat access - Fixes handler Authorize checks to use org-scoped objects with semantically correct actions (ActionUpdate for message/tool operations) - Grants org admins the ability to assign agents-access Closes #24250
johnstcn
force-pushed
the
cian/agents-access-org-role
branch
from
ca9ff54 to
f844481
Compare
|
Went through the changes and pruned out all the unnecessary stuff that got added. |
2 participants
The agents-access role previously granted chat permissions at user scope, but chats are org-scoped objects. Rego skips user-level perms when org_owner is set, making the grants invisible. Handler-level band-aids used synthetic non-org-scoped objects as a workaround.
organization_members.roles (org-scoped) via DB migration
alongside organization-admin, organization-auditor, etc., with
Member permissions granting chat create/read/update
alone no longer grants chat access
semantically correct actions (ActionUpdate for message/tool operations)
Closes #24250
Fixes CODAGT-174
Note: this does not update the "Usage" endpoints. Tracked by CODAGT-161.