Skip to content

[DNM] feat: add chat-access site-wide role to gate chat creation#23724

Draft
johnstcn wants to merge 1 commit intomainfrom
cian/chat-access-role
Draft

[DNM] feat: add chat-access site-wide role to gate chat creation#23724
johnstcn wants to merge 1 commit intomainfrom
cian/chat-access-role

Conversation

@johnstcn
Copy link
Copy Markdown
Member

  • Add chat-access built-in role granting chat CRUD at User scope
  • Exclude ResourceChat from member, org member, and org service account allPermsExcept calls
  • Allow system, owner, and user-admin to assign the new role
  • Migration auto-assigns role to users who have ever created a chat
  • Update frontend role descriptions, display order, and test mocks
  • Update RBAC test matrix: memberMe denied, chatAccessUser allowed
Implementation plan & decisions

Approach: New built-in role (like auditor, template-admin) rather than permission flag on member.

Key decisions:

  • User-scope only — users CRUD their own chats
  • chatAccessUser NOT added to requiredSubjects in test (follows auditor/orgAdminBanWorkspace pattern)
  • Org member + org service account also had ResourceChat excluded to prevent permission leak
  • Down migration removes role unconditionally (lossy but acceptable)
  • userAdmin can assign to stay consistent with user access management

Breaking change: Members without chat-access lose chat creation ability. Migration covers existing chat creators.

🤖 THIS PR HAS NOT YET BEEN REVIEWED BY A HUMAN. DO NOT MERGE YET.

Members no longer get chat CRUD by default. A new built-in
'chat-access' role must be assigned to grant chat create, read,
update, and delete permissions on the user's own chats.

Owners retain full access via site-level permissions. The
system, owner, and user-admin roles can assign chat-access.

A migration auto-assigns the role to every user who has ever
created a chat, so existing users are not disrupted.

Also excludes ResourceChat from org member and org service
account allPermsExcept calls to prevent permission leaks.
@johnstcn johnstcn requested a review from Emyrk as a code owner March 27, 2026 17:01
Copilot AI review requested due to automatic review settings March 27, 2026 17:01
@coder-tasks
Copy link
Copy Markdown
Contributor

coder-tasks bot commented Mar 27, 2026

Documentation Check

Updates Needed

  • docs/admin/users/groups-roles.md - The roles permissions table lists Auditor, User Admin, Template Admin, and Owner, but does not include the new chat-access built-in role. Add a column (or row) for Chat Access describing that it allows creating and using AI chats, and note that members without this role cannot create chats.

New Documentation Needed

  • docs/ai-coder/tasks.md (or a relevant AI coder doc) - The new chat-access role introduces a breaking change for members: users who did not previously create a chat will lose chat creation ability. This should be documented in the AI/Tasks documentation, covering how admins can grant or revoke the role and what the migration does for existing chat users.

Automated review via Coder Tasks

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new built-in chat-access site role to gate chat CRUD/creation, with backend RBAC updates, migration to backfill existing chat creators, and frontend role UI/mocks updates.

Changes:

  • Introduce chat-access built-in role (Go + TS constants) and update RBAC permission matrices/tests.
  • Remove ResourceChat from default member/org-member/org-service-account permission sets to prevent implicit access.
  • Add DB migration to grant chat-access to users who have created chats; update frontend role display/description and story mocks.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
site/src/testHelpers/entities.ts Adds mock chat-access role and includes it in mock role lists.
site/src/pages/OrganizationSettingsPage/UserTable/UserRoleCell.tsx Adds chat-access to the role sort/display ordering.
site/src/pages/OrganizationSettingsPage/UserTable/EditRolesButton.tsx Adds user-facing description for the new role.
site/src/pages/OrganizationSettingsPage/UserTable/EditRolesButton.stories.tsx Updates story to include chat-access in selected roles.
site/src/api/typesGenerated.ts Adds RoleChatAccess constant for frontend use.
codersdk/rbacroles.go Adds RoleChatAccess constant to the SDK.
coderd/rbac/roles_test.go Updates RBAC test matrix to require chat-access for chat CRUD at user scope.
coderd/rbac/roles.go Defines chat-access role; removes ResourceChat from default member/org-member/service-account permission sets; allows assignment by system/owner/user-admin.
coderd/database/migrations/000455_chat_access_role.up.sql Backfills chat-access to users who have created chats.
coderd/database/migrations/000455_chat_access_role.down.sql Removes chat-access from all users on rollback.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

ResourceUser,
ResourceOrganizationMember,
ResourceAibridgeInterception,
ResourceChat,
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding ResourceChat to the OrgMemberPermissions exclusion list removes all org-member-level permissions for the chat resource. However database.ChatFile.RBACObject() uses rbac.ResourceChat...InOrg(c.OrganizationID) (coderd/database/modelmethods.go:181-183), which means chat file CRUD is evaluated under org-member rules. After this change, regular org members (even with the new site-wide chat-access role, which only grants user-scope perms when org_owner == "") will be denied access to chat files.

Consider either (a) authorizing chat file operations as user-scope (don’t set org_owner for chat files / don’t use .InOrg(...)), or (b) introducing a dedicated resource type/permission model for chat files that matches the intended access control.

Suggested change
ResourceChat,

Copilot uses AI. Check for mistakes.
ResourceUser,
ResourceOrganizationMember,
ResourceAibridgeInterception,
ResourceChat,
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same concern as OrgMemberPermissions: excluding ResourceChat here means org service accounts lose all org-member-level access to chat resources. Since ChatFile is modeled as ResourceChat with org_owner set, this will also block service-account interactions with chat files unless chat files are moved to user-scope or otherwise reworked.

Suggested change
ResourceChat,

Copilot uses AI. Check for mistakes.
),
User: append(
allPermsExcept(ResourceWorkspaceDormant, ResourcePrebuiltWorkspace, ResourceWorkspace, ResourceUser, ResourceOrganizationMember, ResourceOrganizationMember, ResourceBoundaryUsage, ResourceAibridgeInterception),
allPermsExcept(ResourceWorkspaceDormant, ResourcePrebuiltWorkspace, ResourceWorkspace, ResourceUser, ResourceOrganizationMember, ResourceOrganizationMember, ResourceBoundaryUsage, ResourceAibridgeInterception, ResourceChat),
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

allPermsExcept is called with ResourceOrganizationMember twice here, which is redundant and makes the exclusion list harder to reason about. Remove the duplicate argument to keep the role definition clean.

Suggested change
allPermsExcept(ResourceWorkspaceDormant, ResourcePrebuiltWorkspace, ResourceWorkspace, ResourceUser, ResourceOrganizationMember, ResourceOrganizationMember, ResourceBoundaryUsage, ResourceAibridgeInterception, ResourceChat),
allPermsExcept(ResourceWorkspaceDormant, ResourcePrebuiltWorkspace, ResourceWorkspace, ResourceUser, ResourceOrganizationMember, ResourceBoundaryUsage, ResourceAibridgeInterception, ResourceChat),

Copilot uses AI. Check for mistakes.
Comment on lines +5902 to +5904
/**
* Ideally this roles would be generated from the rbac/roles.go package.
*/
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The JSDoc comment has a grammatical error: "Ideally this roles" → "Ideally these roles".

Copilot uses AI. Check for mistakes.
@johnstcn johnstcn marked this pull request as draft March 27, 2026 17:07
@johnstcn johnstcn removed the request for review from Emyrk March 27, 2026 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants